From f34de8940ff7d8bb2b7517d1559188886cfca04c Mon Sep 17 00:00:00 2001 From: Mark Veidemanis Date: Thu, 17 Oct 2019 20:19:35 +0100 Subject: [PATCH] Improve performance in userinfo * Implement a nick -> user mapping, preventing a superfluous SSCAN on the entire dataset for when networks are disconnected * Use one thread for all channels when a network instance is disconnected, instead of one thread per channel * Made returns comprising of only a list into tuples --- core/bot.py | 41 ++++------------- modules/userinfo.py | 108 ++++++++++++++++++++++++++------------------ utils/parsing.py | 15 ++++++ 3 files changed, 88 insertions(+), 76 deletions(-) create mode 100644 utils/parsing.py diff --git a/core/bot.py b/core/bot.py index 74e52f2..73e18f8 100644 --- a/core/bot.py +++ b/core/bot.py @@ -21,6 +21,7 @@ import main from utils.logging.log import * from utils.logging.debug import * from utils.logging.send import * +from utils.parsing import parsen from twisted.internet.ssl import DefaultOpenSSLContextFactory @@ -54,20 +55,8 @@ class IRCRelay(IRCClient): self.stage2 = stage2 self.loop = None - def parsen(self, user): - step = user.split("!") - nick = step[0] - if len(step) == 2: - step2 = step[1].split("@") - ident, host = step2 - else: - ident = nick - host = nick - - return [nick, ident, host] - def privmsg(self, user, channel, msg): - nick, ident, host = self.parsen(user) + nick, ident, host = parsen(user) for i in main.ZNCErrors: if i in msg: error("ZNC issue:", msg) @@ -134,22 +123,6 @@ class IRCBot(IRCClient): self.chanlimit = 0 - def parsen(self, user): - step = user.split("!") - nick = step[0] - if len(step) == 2: - step2 = step[1].split("@") - if len(step2) == 2: - ident, host = step2 - else: - ident = nick - host = nick - else: - ident = nick - host = nick - - return [nick, ident, host] - def joinChannels(self, channels): sleeptime = 0.0 increment = 0.8 @@ -178,7 +151,9 @@ class IRCBot(IRCClient): if cast[i] == "": # a dictionary that changes length with each iteration del cast[i] if "muser" in cast.keys(): - cast["nick"], cast["ident"], cast["host"] = self.parsen(cast["muser"]) + cast["nick"], cast["ident"], cast["host"] = parsen(cast["muser"]) + # additional checks here to see if it's a server -- irc.example.net + # discard if it is #if not cast["type"] in ["nick", "kick", "quit", "part", "join"]: # del cast["muser"] if set(["nick", "ident", "host", "msg"]).issubset(set(cast)): @@ -507,7 +482,7 @@ class IRCBot(IRCClient): lc = self._getWho[channel] lc.stop() del self._getWho[channel] - userinfo.delChannel(self.net, channel) # < we do not need to deduplicate this + userinfo.delChannels(self.net, [channel]) # < we do not need to deduplicate this #log("Can no longer cover %s, removing records" % channel)# as it will only be matched once -- # other bots have different nicknames so def left(self, user, channel, message): # even if they saw it, they wouldn't react @@ -529,7 +504,7 @@ class IRCBot(IRCClient): self.event(type="kick", muser=kicker, channel=channel, message=message, user=kickee) def chanlessEvent(self, cast): - cast["nick"], cast["ident"], cast["host"] = self.parsen(cast["muser"]) + cast["nick"], cast["ident"], cast["host"] = parsen(cast["muser"]) if dedup(self.name, cast): # Needs to be kept self.name until the dedup # function is converted to the new net, num # format @@ -590,7 +565,7 @@ class IRCBotFactory(ReconnectingClientFactory): def clientConnectionLost(self, connector, reason): if not self.relay: - userinfo.delNetwork(self.net, self.client.channels) + userinfo.delChannels(self.net, self.client.channels) if not self.client == None: self.client.connected = False self.client.channels = [] diff --git a/modules/userinfo.py b/modules/userinfo.py index a569c42..c8c46cb 100644 --- a/modules/userinfo.py +++ b/modules/userinfo.py @@ -4,12 +4,13 @@ from string import digits import main from utils.logging.log import * from utils.logging.debug import debug +from utils.parsing import parsen def getWhoSingle(name, query): - result = main.r.sscan("live.who."+name, 0, query, count=9999999) + result = main.r.sscan("live.who."+name, 0, query, count=-1) if result[1] == []: return None - return [i.decode() for i in result[1]] + return (i.decode() for i in result[1]) def getWho(query): result = {} @@ -24,14 +25,14 @@ def getChansSingle(name, nick): result = main.r.sinter(*nick) if len(result) == 0: return None - return [i.decode() for i in result] + return (i.decode() for i in result) def getChanList(name, nick): chanspace = "live.chan."+name+"."+nick result = main.r.smembers(chanspace) if len(result) == 0: return None - return [i.decode() for i in result] + return (i.decode() for i in result) def getChans(nick): result = {} @@ -42,11 +43,11 @@ def getChans(nick): return result def getUsersSingle(name, nick): - nick = ["live.who."+name+"."+i for i in nick] + nick = ("live.who."+name+"."+i for i in nick) result = main.r.sinter(*nick) if len(result) == 0: return None - return [i.decode() for i in result] + return (i.decode() for i in result) def getUsers(nick): result = {} @@ -69,13 +70,17 @@ def getNamespace(name, channel, nick): gnamespace = "live.who.%s" % name namespace = "live.who.%s.%s" % (name, channel) chanspace = "live.chan.%s.%s" % (name, nick) - return [gnamespace, namespace, chanspace] + mapspace = "live.map.%s" % name + return (gnamespace, namespace, chanspace, mapspace) def _initialUsers(name, channel, users): gnamespace = "live.who.%s" % name + mapspace = "live.map.%s" % name p = main.r.pipeline() for i in users: - p.sadd(gnamespace, i[0]+"!"+i[1]+"@"+i[2]) + user = i[0]+"!"+i[1]+"@"+i[2] + p.hset(mapspace, i[0], user) + p.sadd(gnamespace, user) p.execute() def initialUsers(name, channel, users): @@ -98,29 +103,36 @@ def initialNames(name, channel, names): def editUser(name, user): gnamespace = "live.who.%s" % name - main.r.sadd(gnamespace, user) + mapspace = "live.map.%s" % name + parsed = parsen(user) + p = main.r.pipeline() + p.sadd(gnamespace, user) + p.hset(mapspace, parsed[0], user) # add nick -> user mapping + p.execute() def addUser(name, channel, nick, user): - gnamespace, namespace, chanspace = getNamespace(name, channel, nick) + gnamespace, namespace, chanspace, mapspace = getNamespace(name, channel, nick) p = main.r.pipeline() p.sadd(gnamespace, user) p.sadd(namespace, nick) p.sadd(chanspace, channel) + p.hset(mapspace, nick, user) p.execute() def delUser(name, channel, nick, user): - gnamespace, namespace, chanspace = getNamespace(name, channel, nick) + gnamespace, namespace, chanspace, mapspace = getNamespace(name, channel, nick) p = main.r.pipeline() channels = main.r.smembers(chanspace) p.srem(namespace, nick) - if channels == {channel.encode()}: - p.delete(chanspace) + if channels == {channel.encode()}: # can we only see them on this channel? + p.delete(chanspace) # remove channel tracking entry + p.hdel(mapspace, nick) # remove nick mapping entry if user: - p.srem(gnamespace, user) + p.srem(gnamespace, user) # remove global userinfo entry else: warn("Attempt to delete nonexistent user: %s" % user) else: - p.srem(chanspace, channel) + p.srem(chanspace, channel) # keep up - remove the channel from their list p.execute() def escape(text): @@ -131,7 +143,14 @@ def escape(text): return text def getUserByNick(name, nick): - gnamespace = "live.who.%s" % name + gnamespace = "live.who.%s" % name # "nick": "nick!ident@host" + mapspace = "live.map.%s" % name + if main.r.hexists(mapspace, nick): + return main.r.hget(mapspace, nick) + else: + warn("Entry doesn't exist: %s on %s - attempting auxiliary lookup" % (nick, mapspace)) + #return Falsedd + # legacy code below - remove when map is reliable usermatch = main.r.sscan(gnamespace, match=escape(nick)+"!*", count=-1) if usermatch[1] == []: return False @@ -140,12 +159,13 @@ def getUserByNick(name, nick): user = usermatch[1][0] return user else: - warn("Entry doesn't exist: %s on %s" % (nick, gnamespace)) + warn("Auxiliary lookup failed: %s on %s" % (nick, gnamespace)) return False def renameUser(name, oldnick, olduser, newnick, newuser): gnamespace = "live.who.%s" % name chanspace = "live.chan.%s.%s" % (name, oldnick) + mapspace = "live.map.%s" % name newchanspace = "live.chan.%s.%s" % (name, newnick) p = main.r.pipeline() p.srem(gnamespace, olduser) @@ -154,51 +174,53 @@ def renameUser(name, oldnick, olduser, newnick, newuser): i = i.decode() p.srem("live.who."+name+"."+i, oldnick) p.sadd("live.who."+name+"."+i, newnick) + p.hdel(mapspace, oldnick) + p.hset(mapspace, newnick, newuser) if main.r.exists(chanspace): p.rename(chanspace, newchanspace) else: warn("Key doesn't exist: %s" % chanspace) p.execute() -def delUserByNick(name, channel, nick): +def delUserByNick(name, channel, nick): # kick user = getUserByNick(name, nick) delUser(name, channel, nick, user) -def delUserByNetwork(name, nick, user): +def delUserByNetwork(name, nick, user): # quit gnamespace = "live.who.%s" % name chanspace = "live.chan.%s.%s" % (name, nick) + mapspace = "live.chan.%s" % name p = main.r.pipeline() p.srem(gnamespace, user) for i in main.r.smembers(chanspace): p.srem("live.who."+name+"."+i.decode(), nick) p.delete(chanspace) + p.hdel(mapspace, nick) p.execute() -def _delChannel(name, channel): # This function is extremely expensive, look to replace - gnamespace = "live.who.%s" % name - namespace = "live.who.%s.%s" % (name, channel) +def _delChannels(net, channels): + gnamespace = "live.who.%s" % net + mapspace = "live.map.%s" % net p = main.r.pipeline() - for i in main.r.smembers(namespace): - user = getUserByNick(name, i.decode()) - if main.r.smembers("live.chan."+name+"."+i.decode()) == {channel.encode()}: - if user: - p.srem(gnamespace, user) - - p.delete("live.chan."+name+"."+i.decode()) - else: - p.srem("live.chan."+name+"."+i.decode(), channel) - p.delete(namespace) + for channel in channels: + namespace = "live.who.%s.%s" % (net, channel) + for i in main.r.smembers(namespace): + nick = i.decode() + #user = getUserByNick(net, nick) -- far too many function calls + user = main.r.hget(mapspace, nick) + if not user: + warn("User lookup failed: %s on %s" % (nick, net)) + if main.r.smembers("live.chan."+net+"."+nick) == {channel.encode()}: + if user: + p.srem(gnamespace, user) + p.delete("live.chan."+net+"."+nick) + p.hdel(mapspace, nick) # remove map entry + else: + p.srem("live.chan."+net+"."+nick, channel) + p.delete(namespace) p.execute() - return [name, channel] -def delChannel(name, channel): - debug("Purging channel %s for %s" % (channel, name)) - d = deferToThread(_delChannel, name, channel) +def delChannels(net, channels): + debug("Purging channel %s for %s" % (", ".join(channels), net)) + d = deferToThread(_delChannels, net, channels) #d.addCallback(testCallback) - -def delNetwork(name, channels): - debug("Purging channels for %s" % name) - for i in channels: - delChannel(name, i) - #log("Finished purging channels for %s" % name) - return diff --git a/utils/parsing.py b/utils/parsing.py new file mode 100644 index 0000000..fb4f588 --- /dev/null +++ b/utils/parsing.py @@ -0,0 +1,15 @@ +def parsen(user): + step = user.split("!") + nick = step[0] + if len(step) == 2: + step2 = step[1].split("@") + if len(step2) == 2: + ident, host = step2 + else: + ident = nick + host = nick + else: + ident = nick + host = nick + + return (nick, ident, host)