Skip to content

The user-label cache is re-parsed from localStorage on every render #5716

Description

@GeneralJah215

Describe the bug

readCache() in desktop/src/features/profile/lib/userLabelStorage.ts runs once
per React render, per query observer. Each run does a localStorage.getItem, a
JSON.parse of up to MAX_CACHED_LABELS (1,000) profile entries, and a full
rebuild of the lowercased profile map. The caller then throws the result away.

Mechanism

readCache is reached through resolveUserLabelPlaceholderData, which
desktop/src/features/profile/hooks.ts:378 hands to React Query as
placeholderData:

placeholderData: (previousData) =>
  resolveUserLabelPlaceholderData(
    previousData,
    relayUrl,
    normalizedPubkeys,
  ),

React Query invokes placeholderData on every result computation — once per
render, per observer. useUsersBatchQuery has roughly 35 call sites across 31
files, and UserProfilePopover calls it twice and mounts per message row, per
avatar, per member-list entry. Every rendered username is an observer.

The worst case is the common one. readCachedUserLabels calls readCache
before it inspects pubkeys:

export function readCachedUserLabels(relayUrl, pubkeys) {
  const cache = readCache(relayUrl);   // full 1,000-entry parse happens here
  if (!cache) return undefined;
  ...                                  // pubkeys not consulted until here
}

UserProfilePopover gates its queries on enabled: isOpen and passes
open ? [pubkey] : []. So while a popover is closed there is no
previousData, the ?? falls through, and a full parse runs to produce a
guaranteed undefined. Two observers per closed popover.

Steps to reproduce

  1. Use Buzz until the label cache is populated — check
    localStorage["buzz-user-labels.v1:<relay>"].
  2. Open a channel with long scrollback so many UserProfilePopovers mount.
  3. Minimize the window. Do not interact with the app.
  4. Attach a CPU profiler to the WebView renderer, or just watch the process.

Quicker check, in the devtools console with the app idle:

const orig = JSON.parse; let n = 0;
JSON.parse = (...a) => { n++; return orig(...a); };
setTimeout(() => { console.log("parses in 10s:", n); JSON.parse = orig; }, 10_000);

Expected behavior

An idle, minimized window should be near 0% CPU. A cache read that returns the
same bytes should not re-parse them.

Measurement

45-second CPU profile of the minimized app over the WebView2 debug port:

[profile] window 46446 ms, 17245 samples

React rendering, total          23.0% of one core
readCache                       largest JS leaf by self time
(garbage collector)              6.7%
main thread idle                66.5%

No hot loop — thousands of small parses. The garbage collector sitting second is
the signature: allocate a 1,000-entry object graph, read one field, drop it,
repeat.

Profiled call chain from the running app, minified names:

readCache
  <- createResult
  <- getOptimisticResult
  <- UserProfilePopover
  <- React scheduler

Version and platform

Additional context

This looks like an oversight rather than a design decision, because the same fix
is already applied one file over. hooks.ts:98 wraps readSelfProfileCache in a
React.useMemo with the comment "Parse localStorage once per relayUrl/pubkey
pair — not on every render"
, and selfProfileStorage.ts carries a module-level
memo. userLabelStorage.ts simply never got the same treatment.

No existing gate catches this: the returned value is correct, only the call
count is wrong. Unit tests, typecheck and lint are all green today.

I have a fix — memoise the parse keyed on the raw string, about six lines, plus a
regression test that asserts the JSON.parse call count rather than the returned
value, because asserting the value passes identically with and without the fix.
Verified against the unfixed file: 25 parses across 25 lookups before, 1 after.
Happy to open a PR if the approach sounds right.

Related but deliberately not bundled: normalizedPubkeys in
useUsersBatchQuery (hooks.ts:325) is rebuilt every render — map + Set +
filter + sort, no useMemo — and it also feeds the query key. Smaller cost,
separate change, happy to file separately.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions