Skip to content

Bug: Perf/message token cache - #901

Open
addyCooks wants to merge 2 commits into
Nano-Collective:mainfrom
addyCooks:perf/message-token-cache
Open

Bug: Perf/message token cache#901
addyCooks wants to merge 2 commits into
Nano-Collective:mainfrom
addyCooks:perf/message-token-cache

Conversation

@addyCooks

Copy link
Copy Markdown
Contributor

Description

Closes #894.

getMessageTokens rebuilt the entire cache on every miss: it allocated a fresh BoundedMap, copied up to 1000 entries into it (each copy also allocating a new entry wrapper), and stored the result back through setMessageTokenCache. Every uncached message cost a full copy of the cache and turned all of its previous entries into garbage.

Because that copy landed in state, it also re-rendered the whole app and handed getMessageTokens a new identity, invalidating the memos and effects keyed on it (context percentage, usage display). And since the write was deferred to a microtask, repeated calls for the same message inside a single render pass all missed usage-display walks every message twice per render each queueing another full copy.

This holds the cache in a ref and writes the entry straight into it. The map instance stays stable for the session, a miss costs one Map.set, the value is visible to the very next lookup, and a cache write no longer reaches React which is what makes the queueMicrotask deferral (added to avoid updating state during render) unnecessary.

Eviction is deliberately unchanged: BoundedMap already drops the oldest entry in O(1) using Map insertion order, so the insertion counter suggested in the issue would add bookkeeping without a win.

setMessageTokenCache is dropped from the returned setters nothing consumed it. messageTokenCache stays exposed.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Changeset

  • Added a changeset (pnpm changeset) describing this change for the changelog

Testing

Automated Tests

  • New features include passing tests in .spec.ts/tsx files
  • All existing tests pass (pnpm test:all completes successfully) — see note below
  • Tests cover both success and error scenarios

Manual Testing

  • Tested with Ollama
  • Tested with OpenRouter
  • Tested with OpenAI-compatible API
  • Tested MCP integration (if applicable)

Checklist

  • If this was for an open issue, I was assigned to it
  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated (if needed)
  • No breaking changes (or clearly documented)
  • Appropriate logging added using structured logging (see CONTRIBUTING.md)

@addyCooks addyCooks changed the title Perf/message token cache Bug: Perf/message token cache Aug 20, 2026
getMessageTokens rebuilt the entire cache on every miss: it allocated a
fresh BoundedMap, copied up to 1000 entries into it (each copy also
allocating a new entry wrapper), and stored the result back through
setMessageTokenCache. Every uncached message therefore cost a full copy
of the cache and turned all of its previous entries into garbage.

Because that copy landed in state it also re-rendered the whole app and
handed getMessageTokens a new identity, invalidating the memos and
effects keyed on it (context percentage, usage display). And since the
write was deferred to a microtask, repeated calls for the same message
inside one render pass all missed, each queueing another full copy.

Hold the cache in a ref and write the entry straight into it. The map
instance stays stable for the session, a miss costs one Map.set, the
value is visible to the very next lookup, and a cache write no longer
reaches React — which is also what makes the queueMicrotask deferral
(added to avoid updating state during render) unnecessary. Eviction is
unchanged: BoundedMap already drops the oldest entry in O(1) using Map
insertion order.

setMessageTokenCache is dropped from the returned setters, nothing
consumed it.

Closes Nano-Collective#894.
@addyCooks
addyCooks force-pushed the perf/message-token-cache branch from eecf88a to c9f2932 Compare August 20, 2026 21:42
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.

[Bug] Performance: In-place BoundedMap Update Causes High Allocation

1 participant