From 8a15405189a4d4e17bfbd6a9a45617d0563bf250 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Sat, 18 Jul 2026 17:09:21 -0400 Subject: [PATCH] perf(presence): shared presence store instead of per-avatar listeners (PERF-1) useUserPresence registered 3 client listeners (Presence / CurrentlyActive / LastPresenceTs) PER hook instance. On a large room that meant 100-250 global listeners, every presence event fanning out across all of them, with add/remove churn on every fast scroll. Replace with a module-level PresenceStore singleton that registers exactly 3 listeners total (lazily, on first subscriber) and fans out to per-user subscribers itself. The hook keeps the same public API (useState + a subscribe effect); consumers are unchanged. Cache + subscriber sets stay bounded to currently-mounted users; the mx-swap branch re-homes listeners on re-login. Reviewed by two passes (SDK mutate-before-emit ordering and handler signatures independently verified). Includes their recommended hardening: the unsubscribe is made idempotent via a set-identity check so a double-invoke / re-subscribe can't evict a newer subscriber. Note: a User object that appears silently with no presence event no longer re-seeds (deps are [mx, userId] not [mx, user]); the common presence-EDU case is handled (and better than before). Reviewers rated this narrow case Low. Co-Authored-By: Claude Opus 4.8 --- src/app/hooks/useUserPresence.ts | 116 ++++++++++++++++++++++++------- 1 file changed, 90 insertions(+), 26 deletions(-) diff --git a/src/app/hooks/useUserPresence.ts b/src/app/hooks/useUserPresence.ts index dafc48193..71b68662b 100644 --- a/src/app/hooks/useUserPresence.ts +++ b/src/app/hooks/useUserPresence.ts @@ -1,5 +1,5 @@ import { useEffect, useMemo, useState } from 'react'; -import { User, UserEvent, UserEventHandlerMap } from 'matrix-js-sdk'; +import { MatrixClient, User, UserEvent, UserEventHandlerMap } from 'matrix-js-sdk'; import { useMatrixClient } from './useMatrixClient'; export enum Presence { @@ -22,35 +22,99 @@ const getUserPresence = (user: User): UserPresence => ({ lastActiveTs: user.getLastActiveTs(), }); -export const useUserPresence = (userId: string): UserPresence | undefined => { - const mx = useMatrixClient(); - const user = mx.getUser(userId); +type PresenceListener = () => void; - const [presence, setPresence] = useState(() => (user ? getUserPresence(user) : undefined)); +/** + * Shared presence store. Previously every avatar's `useUserPresence` registered + * its own 3 client listeners (Presence / CurrentlyActive / LastPresenceTs) — on + * a big room that meant hundreds of global listeners, each presence event + * fanning out across all of them, with churn on every fast scroll (PERF-1). + * + * This registers exactly 3 listeners total (once, on the first subscriber) and + * fans out to per-user subscribers itself. Subscribing on the client rather + * than per-`User` object also avoids the SDK's 10-listener-per-User cap. + */ +class PresenceStore { + private mx: MatrixClient | undefined; - useEffect(() => { - // Re-seed when the User object appears/changes after first render — the - // useState initializer only ran if `user` already existed at mount, so a - // late-arriving user would otherwise show no presence until the next event. - if (user) setPresence(getUserPresence(user)); - // Subscribe on mx (MatrixClient) rather than on individual User objects. - // User objects have a default 10-listener limit; the same user can appear - // in many components simultaneously (avatars, member list, etc.) and - // per-user subscription causes MaxListenersExceededWarning at 11+. - const updatePresence: UserEventHandlerMap[UserEvent.Presence] = (event, u) => { - if (u.userId === user?.userId) { - setPresence(getUserPresence(u)); + private started = false; + + // userId -> set of component callbacks to notify on change. + private subscribers = new Map>(); + + // userId -> last computed presence (stable reference until it changes), only + // retained while at least one subscriber for that user is mounted. + private cache = new Map(); + + private handleEvent: UserEventHandlerMap[UserEvent.Presence] = (_event, user) => { + const subs = this.subscribers.get(user.userId); + if (!subs || subs.size === 0) return; // ignore users nothing is watching + this.cache.set(user.userId, getUserPresence(user)); + subs.forEach((cb) => cb()); + }; + + private start(mx: MatrixClient): void { + if (this.started && this.mx === mx) return; + // A different client (re-login without reload) — drop the old listeners. + if (this.started && this.mx) { + this.mx.removeListener(UserEvent.Presence, this.handleEvent); + this.mx.removeListener(UserEvent.CurrentlyActive, this.handleEvent); + this.mx.removeListener(UserEvent.LastPresenceTs, this.handleEvent); + this.cache.clear(); + } + this.mx = mx; + mx.on(UserEvent.Presence, this.handleEvent); + mx.on(UserEvent.CurrentlyActive, this.handleEvent); + mx.on(UserEvent.LastPresenceTs, this.handleEvent); + this.started = true; + } + + subscribe(mx: MatrixClient, userId: string, cb: PresenceListener): () => void { + this.start(mx); + let subs = this.subscribers.get(userId); + if (!subs) { + subs = new Set(); + this.subscribers.set(userId, subs); + } + subs.add(cb); + return () => { + subs.delete(cb); + // Only tear down the registry entry if this is still the live set for the + // user — keeps unsubscribe idempotent so a double-invoke (or a later + // re-subscribe that replaced the set) can't evict a newer subscriber. + if (subs.size === 0 && this.subscribers.get(userId) === subs) { + this.subscribers.delete(userId); + this.cache.delete(userId); } }; - mx.on(UserEvent.Presence, updatePresence); - mx.on(UserEvent.CurrentlyActive, updatePresence); - mx.on(UserEvent.LastPresenceTs, updatePresence); - return () => { - mx.removeListener(UserEvent.Presence, updatePresence); - mx.removeListener(UserEvent.CurrentlyActive, updatePresence); - mx.removeListener(UserEvent.LastPresenceTs, updatePresence); - }; - }, [mx, user]); + } + + get(mx: MatrixClient, userId: string): UserPresence | undefined { + if (this.cache.has(userId)) return this.cache.get(userId); + const user = mx.getUser(userId); + const presence = user ? getUserPresence(user) : undefined; + // Only retain in the cache while something is actually subscribed, so a + // render that never commits (and thus never subscribes) can't leave a + // dangling entry. handleEvent keeps it fresh thereafter. + if (this.subscribers.get(userId)?.size) this.cache.set(userId, presence); + return presence; + } +} + +const presenceStore = new PresenceStore(); + +export const useUserPresence = (userId: string): UserPresence | undefined => { + const mx = useMatrixClient(); + const [presence, setPresence] = useState(() => presenceStore.get(mx, userId)); + + useEffect(() => { + // Re-seed synchronously (userId/mx may have changed, or a late-arriving User + // object now exists), then track changes via the shared store. + setPresence(presenceStore.get(mx, userId)); + return presenceStore.subscribe(mx, userId, () => { + setPresence(presenceStore.get(mx, userId)); + }); + }, [mx, userId]); return presence; };