Reword GatewayMUCMembership storage to significantly speed up common operations #293
Conversation
tadzik
force-pushed
the
tadzik/speedup-joins
branch
from
f57b8ae
to
5d2759e
Compare
…operations The biggest winners here are addMatrixMember() and removeMatrixMember(), which were a hotpath of updateMatrixMemberListForRoom, called upon xmpp users joining matrix rooms, which used to lock up the process completely for large rooms. On a local test with a 3000-person room, the joins went down from ~500ms to ~200 and then ~50 once the caches warm up. The remaining time *seems* to be mostly spent on IO (sending actual XMPP presence updates), and the offending code disappears completely from a profiler output – hopefully meaning that the bridge will have ample time to deal with other tasks instead of locking up completely like it used to. There are some remaining bits that iterate on indexable member lists, though they don't seem to be a measurable performance problem for now, so I left them as they are – but easily identifyable with the menacing Array.from().
tadzik
force-pushed
the
tadzik/speedup-joins
branch
from
5d2759e
to
4e3f8cf
Compare
| @@ -49,11 +73,14 @@ export class GatewayMUCMembership { | |||
| } | |||
|
|
|||
| public getMemberByAnonJid<G extends IGatewayMember>(chatName: string, anonJid: string): G|undefined { | |||
| return this.getMembers(chatName).find((user) => user.anonymousJid.toString() === anonJid) as G; | |||
| return Array.from(this.getMembers(chatName)).find((user) => user.anonymousJid.toString() === anonJid) as G; | |||
There was a problem hiding this comment.
I can't believe JS doesn't support find/filter on iterators – straight up encouraging you to create this sort of giant Arrays just to throw them away a moment later.
I didn't add more indexes to ChatMembers for these though, since I was unable to make these functions show up in the profiler output at all (it drops everything that didn't exceed 1% of its parent's runtime).
There was a problem hiding this comment.
The 1% is fine but yeah the inability to iterate on sets is, cough, awful. Almost tempted to use a utility library...
There was a problem hiding this comment.
Yeah, I was looking for something in our existing dependency chain, but nothing caught me eye. Someday... :)
changelog.d/293.misc
Outdated
| @@ -0,0 +1 @@ | |||
| Speed up joins for large rooms, preventing them from locking up the process | |||
There was a problem hiding this comment.
(for xmpp gateways only)
| @@ -49,11 +73,14 @@ export class GatewayMUCMembership { | |||
| } | |||
|
|
|||
| public getMemberByAnonJid<G extends IGatewayMember>(chatName: string, anonJid: string): G|undefined { | |||
| return this.getMembers(chatName).find((user) => user.anonymousJid.toString() === anonJid) as G; | |||
| return Array.from(this.getMembers(chatName)).find((user) => user.anonymousJid.toString() === anonJid) as G; | |||
There was a problem hiding this comment.
The 1% is fine but yeah the inability to iterate on sets is, cough, awful. Almost tempted to use a utility library...
The biggest winners here are addMatrixMember() and removeMatrixMember(),
which were a hotpath of updateMatrixMemberListForRoom, called upon xmpp users
joining matrix rooms, which used to lock up the process completely for
large rooms.
On a local test with a 3000-person room, the joins went down from ~500ms
to ~200 and then ~50 once the caches warm up. The remaining time seems
to be mostly spent on IO (sending actual XMPP presence updates), and the
offending code disappears completely from a profiler output – hopefully
meaning that the bridge will have ample time to deal with other tasks
instead of locking up completely like it used to.
There are some remaining bits that iterate on indexable member lists,
though they don't seem to be a measurable performance problem for now,
so I left them as they are – but easily identifyable with the menacing
Array.from().