perf(messages): stop re-rendering every row on each channel-list publish - #473
Conversation
The store replaces the channel list on every message in any channel, and every mounted message row subscribed to the whole list. Rows, message management items and channel identity names now subscribe to the one roster fact they read through useListedChannel, and the reference directory keeps its channel array while the fields a reference reads are unchanged. The rich composer input subscribes to its own channel instead of relying on the directory subscription for membership changes. Also on the per-message path: unread evidence carries its channel and reply root instead of re-deriving them per selector, thread activity is folded only when there is unread thread activity, and the history byte budget sizes each retained event once instead of re-serializing the whole window. Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Signed-off-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
No changes requested; I found no actionable defect in the reviewed diff. Star Lord automated source review via Wes’s account: head fd7bfb5b964fe644546af81bcc700c80b3616a64, base 8562c29c4eef92a229b565d84a94a136fb20f801. Source-only: no tests or app workflows executed, and the reported performance gains were not independently validated. One hosted CI snapshot showed JavaScript and browser measurements passing, with one WebKit journey shard still running.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking findings at fd7bfb5b. Yes, this helps, and it is a good, proportionate change. The implementation meets the 9/10 bar; the advertised percentage improvement is not established for this exact patch.
- Real avoided work: an independent mounted
MessageRowprobe delivered 200 background-style list publications with stable props. The pre-change UI modules produced 200 additional row renders; this head produced zero. Read-only/archive transitions, changed membership, channel removal and session replacement still updated correctly. The reference directory retained activity-only snapshots and refreshed changed labels/privacy/archive state. An independent unread-owner differential probe passed 121 checkpoints and reduced full folds from 100 to 0 when no eligible unread thread activity existed. - Appropriate design: narrow snapshots address the dependency causing the renders rather than adding another store or throttling correctness. Event-size/channel-tag memoization matches the existing frozen-event contract. Byte-size probes matched full UTF-8 JSON serialization, including tail wrappers, while serializing 2,600 distinct objects once across 200 growing-window checks instead of repeatedly serializing 500,100 retained entries. This still scans cached sizes; unread activity still performs a full fold when eligible items exist. Composer summary subscriptions, management context churn and sidebar work remain.
- Validation and limits: hosted JavaScript CI passed 5,727/5,727 tests on merge
274d9788(fd7bfb5b+8562c29c). All journey shards and the required gate are green; the measurement artifact has 9/9 cases plus the classic-scrollbar case passing. Windows validation was skipped. My three focused head probes passed, but they are synthetic jsdom/helper checks, not browser/native speed measurements. The author’s older-base, extra-timestamp-cache benchmark is useful directional evidence, not an exact-PR 39–87% claim. Optional: retain the burst harness and rerun interleaved controls on the exact patch before quoting those percentages. No new native or human acceptance is attested, and this comment is not an approval.
…followup * origin/main: fix(sidebar): save channel moves on desktop and move channels by drag (#474) perf(messages): stop re-rendering every row on each channel-list publish (#473) Unify workspace panels and add persistent channel tabs (#413) ci: prune expired preview releases after promotion (#461) feat(sidebar): show unread conversation counts and DM avatar previews (#472) fix(previews): dismiss hovered previews when their trigger scrolls (#460) perf(native): reduce crypto, upload and discovery overhead (#464) Signed-off-by: Tree Trunks <6ba22921d9dc2ad0aa6ecdf63787ddd24726e266d866da31af69f2e4e146ace5@buzz.block.builderlab.xyz> # Conflicts: # src/shared/design-system/icons/index.ts # tests/browser/layout.spec.mjs
Problem
The relay store replaces the channel list on every message in any channel. Every mounted
MessageRowsubscribed to the whole list, as didMessageManagementItems,useChannelIdentityNamesanduseReferenceDirectory. So every mounted row re-rendered for every message, including messages in channels the user is not viewing.Two smaller costs sat on the same per-message path:
Changes
relay/listed-channel.ts(new):useListedChannel(channels, id, select)subscribes to one fact about one channel and re-renders only when that fact changes.MessageRow,MessageManagementItemsanduseChannelIdentityNamesuse it.ReferenceText.tsx: the reference directory keeps its previous channel array while the fields a reference reads (id, name, type, private, archived) are unchanged.RichComposerInput.tsx: subscribes to its own channel. It had been picking up membership changes only through the directory's subscription.unread.ts: evidence carries its channel and reply root. Thread activity is folded only when there is unread thread activity.budget.ts/store.ts:listByteSizesizes each retained event once. Events are frozen after verification, so the total matchesbyteSize.Sidebar code is untouched.
Measurements
Production web build, headless Chromium, fixture relay. Base and changed trees were interleaved over 3 rounds. Values are median [min..max].
Caveats on these numbers:
bfe4c7f1with one extra change that is not in this PR: a cache for theIntl.DateTimeFormatinstances inMessageTimestamp. That cache accounted for 31–62 ms of self time per live burst in CPU profiles, so the two open-channel rows overstate this PR by roughly that much. The background rows do not render timestamps and are unaffected. The table was not re-measured without the cache.A dev-build trace recorded at this commit in real use shows 24
MessageRowrenders against 4,950ChannelSidebarRowrenders over 94 s; message rows account for 0.2% of sampled script time.Tests
relay/listed-channel.test.tsx(new): no render when another channel changes or when an unselected field changes.relay/budget.test.ts(new):listByteSizeequalsbyteSize.Validation at
fd7bfb5bbiome check --error-on-warnings src,tsc --noEmit,design:typecheck,design:check: pass.vitest run(full): 5648 of 5714 passed in one run at load average 45–74; 8 tests failed with timeouts. Every file that failed passes when re-run with two workers (the last six files: 121/121). There was no single clean full run at this commit. The same source plus the timestamp cache passed 5698/5698 on the earlier base.BUZZ_TEST_WORKERS=2. A first attempt at default worker count had 13 timeouts at load average 65.