diff --git a/irc/channel.go b/irc/channel.go index ca8e45f1..258d3568 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -34,7 +34,6 @@ type Channel struct { key string forward string members MemberSet - membersCache []*Client // allow iteration over channel members without holding the lock name string nameCasefolded string server *Server @@ -54,6 +53,9 @@ type Channel struct { dirtyBits uint settings ChannelSettings uuid utils.UUID + // these caches are paired to allow iteration over channel members without holding the lock + membersCache []*Client + memberDataCache []*memberData } // NewChannel creates a new channel from a `Server` and a `name` @@ -421,16 +423,19 @@ func (channel *Channel) AcceptTransfer(client *Client) (err error) { func (channel *Channel) regenerateMembersCache() { channel.stateMutex.RLock() - result := make([]*Client, len(channel.members)) + membersCache := make([]*Client, len(channel.members)) + dataCache := make([]*memberData, len(channel.members)) i := 0 - for client := range channel.members { - result[i] = client + for client, info := range channel.members { + membersCache[i] = client + dataCache[i] = info i++ } channel.stateMutex.RUnlock() channel.stateMutex.Lock() - channel.membersCache = result + channel.membersCache = membersCache + channel.memberDataCache = dataCache channel.stateMutex.Unlock() } @@ -438,6 +443,8 @@ func (channel *Channel) regenerateMembersCache() { func (channel *Channel) Names(client *Client, rb *ResponseBuffer) { channel.stateMutex.RLock() clientData, isJoined := channel.members[client] + chname := channel.name + membersCache, memberDataCache := channel.membersCache, channel.memberDataCache channel.stateMutex.RUnlock() isOper := client.HasRoleCapabs("sajoin") respectAuditorium := channel.flags.HasMode(modes.Auditorium) && !isOper && @@ -445,52 +452,32 @@ func (channel *Channel) Names(client *Client, rb *ResponseBuffer) { isMultiPrefix := rb.session.capabilities.Has(caps.MultiPrefix) isUserhostInNames := rb.session.capabilities.Has(caps.UserhostInNames) - maxNamLen := 480 - len(client.server.name) - len(client.Nick()) - var namesLines []string - var buffer strings.Builder + maxNamLen := 480 - len(client.server.name) - len(client.Nick()) - len(chname) + var tl utils.TokenLineBuilder + tl.Initialize(maxNamLen, " ") if isJoined || !channel.flags.HasMode(modes.Secret) || isOper { - for _, target := range channel.Members() { + for i, target := range membersCache { + if !isJoined && target.HasMode(modes.Invisible) && !isOper { + continue + } var nick string if isUserhostInNames { nick = target.NickMaskString() } else { nick = target.Nick() } - channel.stateMutex.RLock() - memberData, _ := channel.members[target] - channel.stateMutex.RUnlock() - modeSet := memberData.modes - if modeSet == nil { + memberData := memberDataCache[i] + if respectAuditorium && memberData.modes.HighestChannelUserMode() == modes.Mode(0) { continue } - if !isJoined && target.HasMode(modes.Invisible) && !isOper { - continue - } - if respectAuditorium && modeSet.HighestChannelUserMode() == modes.Mode(0) { - continue - } - prefix := modeSet.Prefixes(isMultiPrefix) - if buffer.Len()+len(nick)+len(prefix)+1 > maxNamLen { - namesLines = append(namesLines, buffer.String()) - buffer.Reset() - } - if buffer.Len() > 0 { - buffer.WriteString(" ") - } - buffer.WriteString(prefix) - buffer.WriteString(nick) - } - if buffer.Len() > 0 { - namesLines = append(namesLines, buffer.String()) + tl.AddParts(memberData.modes.Prefixes(isMultiPrefix), nick) } } - for _, line := range namesLines { - if buffer.Len() > 0 { - rb.Add(nil, client.server.name, RPL_NAMREPLY, client.nick, "=", channel.name, line) - } + for _, line := range tl.Lines() { + rb.Add(nil, client.server.name, RPL_NAMREPLY, client.nick, "=", chname, line) } - rb.Add(nil, client.server.name, RPL_ENDOFNAMES, client.nick, channel.name, client.t("End of NAMES list")) + rb.Add(nil, client.server.name, RPL_ENDOFNAMES, client.nick, chname, client.t("End of NAMES list")) } // does `clientMode` give you privileges to grant/remove `targetMode` to/from people, @@ -514,7 +501,7 @@ func channelUserModeHasPrivsOver(clientMode modes.Mode, targetMode modes.Mode) b // ClientIsAtLeast returns whether the client has at least the given channel privilege. func (channel *Channel) ClientIsAtLeast(client *Client, permission modes.Mode) bool { channel.stateMutex.RLock() - memberData := channel.members[client] + memberData, present := channel.members[client] founder := channel.registeredFounder channel.stateMutex.RUnlock() @@ -522,6 +509,10 @@ func (channel *Channel) ClientIsAtLeast(client *Client, permission modes.Mode) b return true } + if !present { + return false + } + for _, mode := range modes.ChannelUserModes { if memberData.modes.HasMode(mode) { return true @@ -571,20 +562,20 @@ func (channel *Channel) setMemberStatus(client *Client, status alwaysOnChannelSt } channel.stateMutex.Lock() defer channel.stateMutex.Unlock() - if _, ok := channel.members[client]; !ok { - return + if mData, ok := channel.members[client]; ok { + mData.modes.Clear() + for _, mode := range status.Modes { + mData.modes.SetMode(modes.Mode(mode), true) + } + mData.joinTime = status.JoinTime } - memberData := channel.members[client] - memberData.modes = newModes - memberData.joinTime = status.JoinTime - channel.members[client] = memberData } func (channel *Channel) ClientHasPrivsOver(client *Client, target *Client) bool { channel.stateMutex.RLock() founder := channel.registeredFounder - clientModes := channel.members[client].modes - targetModes := channel.members[target].modes + clientData, clientOK := channel.members[client] + targetData, targetOK := channel.members[target] channel.stateMutex.RUnlock() if founder != "" { @@ -595,7 +586,11 @@ func (channel *Channel) ClientHasPrivsOver(client *Client, target *Client) bool } } - return channelUserModeHasPrivsOver(clientModes.HighestChannelUserMode(), targetModes.HighestChannelUserMode()) + return clientOK && targetOK && + channelUserModeHasPrivsOver( + clientData.modes.HighestChannelUserMode(), + targetData.modes.HighestChannelUserMode(), + ) } func (channel *Channel) hasClient(client *Client) bool { @@ -1319,9 +1314,9 @@ func (channel *Channel) SendSplitMessage(command string, minPrefixMode modes.Mod if channel.flags.HasMode(modes.OpModerated) { channel.stateMutex.RLock() - cuData := channel.members[client] + cuData, ok := channel.members[client] channel.stateMutex.RUnlock() - if cuData.modes.HighestChannelUserMode() == modes.Mode(0) { + if !ok || cuData.modes.HighestChannelUserMode() == modes.Mode(0) { // max(statusmsg_minmode, halfop) if minPrefixMode == modes.Mode(0) || minPrefixMode == modes.Voice { minPrefixMode = modes.Halfop @@ -1490,6 +1485,7 @@ func (channel *Channel) Purge(source string) { chname := channel.name members := channel.membersCache channel.membersCache = nil + channel.memberDataCache = nil channel.members = make(MemberSet) // TODO try to prevent Purge racing against (pending) Join? channel.stateMutex.Unlock() diff --git a/irc/client_test.go b/irc/client_test.go index 64d1036c..ca4c2619 100644 --- a/irc/client_test.go +++ b/irc/client_test.go @@ -4,8 +4,10 @@ package irc import ( + "fmt" "testing" + "github.com/ergochat/ergo/irc/languages" "github.com/ergochat/ergo/irc/utils" ) @@ -30,6 +32,47 @@ func BenchmarkGenerateBatchID(b *testing.B) { } } +func BenchmarkNames(b *testing.B) { + channelSize := 1024 + server := &Server{ + name: "ergo.test", + } + lm, err := languages.NewManager(false, "", "") + if err != nil { + b.Fatal(err) + } + server.config.Store(&Config{ + languageManager: lm, + }) + for i := 0; i < b.N; i++ { + channel := &Channel{ + name: "#test", + nameCasefolded: "#test", + server: server, + members: make(MemberSet), + } + for j := 0; j < channelSize; j++ { + nick := fmt.Sprintf("client_%d", j) + client := &Client{ + server: server, + nick: nick, + nickCasefolded: nick, + } + channel.members.Add(client) + channel.regenerateMembersCache() + session := &Session{ + client: client, + } + rb := NewResponseBuffer(session) + channel.Names(client, rb) + if len(rb.messages) < 2 { + b.Fatalf("not enough messages: %d", len(rb.messages)) + } + // to inspect the messages: line, _ := rb.messages[0].Line() + } + } +} + func TestUserMasks(t *testing.T) { var um UserMaskSet diff --git a/irc/handlers.go b/irc/handlers.go index 31995924..043519de 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2060,8 +2060,6 @@ func namesHandler(server *Server, client *Client, msg ircmsg.Message, rb *Respon channels = strings.Split(msg.Params[0], ",") } - // TODO: in a post-federation world, process `target` (server to forward request to) - // implement the modern behavior: https://modern.ircdocs.horse/#names-message // "Servers MAY only return information about the first and silently ignore the others." // "If no parameter is given for this command, servers SHOULD return one RPL_ENDOFNAMES numeric diff --git a/irc/modes/modes.go b/irc/modes/modes.go index 1024c8ed..ad160169 100644 --- a/irc/modes/modes.go +++ b/irc/modes/modes.go @@ -345,6 +345,10 @@ func NewModeSet() *ModeSet { return &set } +func (set *ModeSet) Clear() { + utils.BitsetClear(set[:]) +} + // test whether `mode` is set func (set *ModeSet) HasMode(mode Mode) bool { if set == nil { diff --git a/irc/types.go b/irc/types.go index 45da2536..a4484d46 100644 --- a/irc/types.go +++ b/irc/types.go @@ -16,17 +16,16 @@ import ( type ClientSet = utils.HashSet[*Client] type memberData struct { - modes *modes.ModeSet + modes modes.ModeSet joinTime int64 } // MemberSet is a set of members with modes. -type MemberSet map[*Client]memberData +type MemberSet map[*Client]*memberData // Add adds the given client to this set. func (members MemberSet) Add(member *Client) { - members[member] = memberData{ - modes: modes.NewModeSet(), + members[member] = &memberData{ joinTime: time.Now().UnixNano(), } } diff --git a/irc/utils/bitset.go b/irc/utils/bitset.go index 2e478a69..c90bd7ca 100644 --- a/irc/utils/bitset.go +++ b/irc/utils/bitset.go @@ -48,6 +48,13 @@ func BitsetSet(set []uint32, position uint, on bool) (changed bool) { } } +// BitsetClear clears the bitset in-place. +func BitsetClear(set []uint32) { + for i := 0; i < len(set); i++ { + atomic.StoreUint32(&set[i], 0) + } +} + // BitsetEmpty returns whether the bitset is empty. // This has false positives under concurrent modification (i.e., it can return true // even though w.r.t. the sequence of atomic modifications, there was no point at diff --git a/irc/utils/text.go b/irc/utils/text.go index 7a76b371..c42368c6 100644 --- a/irc/utils/text.go +++ b/irc/utils/text.go @@ -125,6 +125,28 @@ func (t *TokenLineBuilder) Add(token string) { t.buf.WriteString(token) } +// AddParts concatenates `parts` into a token and adds it to the line, +// creating a new line if necessary. +func (t *TokenLineBuilder) AddParts(parts ...string) { + var tokenLen int + for _, part := range parts { + tokenLen += len(part) + } + if t.buf.Len() != 0 { + tokenLen += len(t.delim) + } + if t.lineLen < t.buf.Len()+tokenLen { + t.result = append(t.result, t.buf.String()) + t.buf.Reset() + } + if t.buf.Len() != 0 { + t.buf.WriteString(t.delim) + } + for _, part := range parts { + t.buf.WriteString(part) + } +} + // Lines terminates the line-building and returns all the lines. func (t *TokenLineBuilder) Lines() (result []string) { result = t.result diff --git a/irc/utils/text_test.go b/irc/utils/text_test.go index 6aa05e76..a0b4ca45 100644 --- a/irc/utils/text_test.go +++ b/irc/utils/text_test.go @@ -43,3 +43,26 @@ func TestBuildTokenLines(t *testing.T) { val = BuildTokenLines(10, []string{"abcd", "efgh", "ijkl"}, ",") assertEqual(val, []string{"abcd,efgh", "ijkl"}, t) } + +func TestTLBuilderAddParts(t *testing.T) { + var tl TokenLineBuilder + tl.Initialize(20, " ") + tl.Add("bob") + tl.AddParts("@", "alice") + tl.AddParts("@", "ErgoBot__") + assertEqual(tl.Lines(), []string{"bob @alice", "@ErgoBot__"}, t) +} + +func BenchmarkTokenLines(b *testing.B) { + tokens := strings.Fields(monteCristo) + b.ResetTimer() + + for i := 0; i < b.N; i++ { + var tl TokenLineBuilder + tl.Initialize(400, " ") + for _, tok := range tokens { + tl.Add(tok) + } + tl.Lines() + } +}