Skip to content

perf(mentions): name each choice set once and skip closed choosers - #310

Open
kalvinnchau wants to merge 1 commit into
mainfrom
peon/mention-name-performance
Open

kalvinnchau wants to merge 1 commit into
mainfrom
peon/mention-name-performance

Conversation

@kalvinnchau

Copy link
Copy Markdown
Contributor

Problem

When useMentionChoices builds the list of mention candidates, it resolves each row with names.resolve(pubkey, name, keys, facts). The directory normalizes and sorts the whole candidate list again on every call, and its one-entry cache keeps swapping between scoped and unscoped lookups. As a result, naming a choice set costs roughly O(n² log n). The hook also builds candidates while the picker is closed or disabled.

Change

  • identity-names/directory.ts: the provider contract resolve + qualifier is replaced by a single scope(source, candidates, facts) → pubkey → {name, qualifier} call. The cache now has two layers:

    • a base layer, keyed on library, native, profiles, relay and viewer;
    • an 8-entry LRU of policy results, keyed by the normalized candidate set plus the displayFacts reference.

    This way, interleaved scoped and unscoped lookups keep their own entries. Outside keys still join only their own selection.

  • identity-names/service.ts: the view gains scope(candidates, facts), and lookup/resolve go through it. A held scope follows provider swaps and returns undefined after dispose.

  • use-mention-choices.ts: one names.scope() per choice set replaces the per-row resolve. The hook returns no candidates while the chooser is closed or disabled.

  • mention-candidates.ts: a Set replaces members.includes for membership.

The naming semantics are unchanged: historical nonmember lookups, candidate-specific ambiguity, hidden native/library collisions, invalidation on source/viewer/fact changes, and archived filtering.

Evidence

Choice-set naming step (the naming work in useMentionChoices' current()), median of 9 runs, 5% namesakes, plus 1,000 cached nonmembers and 200 library agents:

members main 401fb8d3 this branch
500 86.0 ms 1.9 ms
2000 1,350.7 ms 5.4 ms

Tests:

  • New regressions:
    • policy runs per scope across mixed case, outside keys and updates;
    • a hot scope surviving 8 interleaved historical lookups (this one fails without the LRU refresh);
    • no names.scope call while the picker is closed or disabled (fails without the guard);
    • namesakes qualified in both the picker and inline completion.
  • vitest run: 4345/4346 passed. The one failure, dev/vite-config.test.mjs, passes when run alone.
  • Browser specs mentions, mention-edit, mention-rules, typeahead, completion-work, completion-layout and new-message in Chromium and WebKit: 74/74 passed.
  • tsc --noEmit and biome check --error-on-warnings src are clean.

Not verified

  • No hosted large-roster before/after app profile. The numbers above are the isolated naming step, not a running-app trace.
  • Git hooks were not installed locally (install-hooks refuses because of a global core.hooksPath), so the equivalent checks were run by hand.

useMentionChoices resolved every row through names.resolve, which
normalized the whole candidate list per row, and it built choices even
while the picker was closed or disabled.

- Replace the provider's resolve + qualifier pair with one scope(source,
  candidates, facts) call, and keep a small LRU of candidate scopes
  above a base layer keyed on the name sources, so scoped and unscoped
  lookups no longer evict each other.
- Add names.scope() to the view; useMentionChoices resolves one scope
  per choice set instead of one normalization per row.
- Return no candidates while the chooser is closed or disabled.
- Use a Set for channel membership in mentionCandidates.

Co-authored-by: Kalvin Chau <kalvin@block.xyz>
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
Signed-off-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz>
@kalvinnchau
kalvinnchau requested review from a team, comp615 and wesbillman as code owners September 26, 2026 14:53

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Star Lord automated source review

Published via Wes’s account (wesbillman). Reviewed head 98d6abcd565ff0d620338a197ecac579e117baec against base 401fb8d301a310ea7d12fc1a59c51bf1907c2078.

No actionable findings in this source review.

Traced the nine-file diff and supported callers, including:

  • Candidate normalization, outside-key historical lookups, bounded LRU refresh/eviction, display-fact replacement, and invalidation on library/native/profile/community/viewer changes.
  • Provider replacement/disable, public-profile fallback, and disposal; the public naming-policy registration remains unchanged.
  • Toolbar closed/disabled/reopened states and inline completion, preserving candidate-local ambiguity, exact recipient keys, membership/archive filtering, and selection-time rechecks.

The change removes repeated candidate normalization from the mention choice-set naming path without changing the underlying naming policy or adding an eligibility owner.

Validation limits: source-only; I did not execute PR code, tests, builds, benchmarks, or the app. The reported speedups and local test results are the author’s evidence, not independently reproduced measurements. A single hosted-check snapshot showed JavaScript, browser measurements, DCO and security checks passing; six browser shards and Rust/tool integration were still running, and Windows native validation was skipped. This is not an all-green CI, end-to-end performance, runtime-acceptance, or merge-approval claim.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants