Pass-scoped compute memo #4926
Conversation
Reduces redundant `computeVia` work during the per-card prerender: - `BaseDef.[queryableValue]` no longer re-reads `value[fieldName]` after `peekAtField` — the second read re-invoked `computeVia` for every contains/contains-many/links-to field in the search doc. - `beginComputePass` / `endComputePass` install a synchronous per-instance compute memo around `serializeCard + searchDoc` so a card with N computeds runs each one once per render.meta pass instead of once per traversal. - The prerender server's two-shot render.meta capture is collapsed to a single call. `model.capturedDeps` is frozen at parent readyPromise resolution and the fitted/embedded renders that ran between the two captures don't mutate the card model, so the second call was emitting the same bytes as the first. Host emits `computedCalls`, `computedCacheHits`, `serializeMs`, `searchDocMs` per row; render-settlement lifts them onto `response.meta.diagnostics` so they persist into `boxel_index.timing_diagnostics` for SQL-side perf triage. The indexing-diagnostics skill documents the new fields and adds a SQL pattern for ranking rows by computed-call pressure. Off-pass reads pay a single `if (passComputeMemo === null)` branch in `getter`, no counter increment and no Map operations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Preview deploymentsHost Test Results 1 files ± 0 1 suites ±0 1h 47m 31s ⏱️ - 1m 22s Results for commit 0cc300f. ± Comparison against earlier commit 2a472dd. Realm Server Test Results 1 files ±0 1 suites ±0 10m 27s ⏱️ -26s Results for commit 0cc300f. ± Comparison against earlier commit 2a472dd. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2102efa80f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR optimizes prerender performance by (1) memoizing computed-field (computeVia) reads within a single render.meta traversal and (2) removing a redundant second render.meta call during per-card prerender. It also plumbs host-side computed/timing diagnostics through to the prerender/indexing diagnostics payload for SQL-side analysis.
Changes:
- Add a synchronous “compute pass” memo in
baseto collapse duplicate computed-field reads duringserializeCard + searchDoc. - Update host
render.metato wrap traversals in the compute pass and emitcomputedCalls/computedCacheHits/serializeMs/searchDocMsdiagnostics. - Deduplicate prerender server’s double
render.metainvocation and lift success-path diagnostics into persisted timing diagnostics.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/base/field-support.ts | Introduces pass-scoped computed memo + counters via beginComputePass/endComputePass, used by getter() for computed fields. |
| packages/base/card-api.gts | Re-exports compute pass APIs and fixes a double-read in [queryableValue] by reusing the peeked value. |
| packages/host/app/routes/render/meta.ts | Opens/closes compute pass around serializeCard + searchDoc, measures timings, logs, and attaches diagnostics to PrerenderMeta. |
| packages/realm-server/prerender/render-runner.ts | Removes the second render.meta call and reuses the single result for both ancestor rendering and final response. |
| packages/realm-server/prerender/render-settlement.ts | Lifts success-path card.diagnostics into response.meta.diagnostics so it persists to timing diagnostics. |
| packages/runtime-common/index.ts | Adds PrerenderMetaDiagnostics and extends PrerenderMeta / RenderTimeoutDiagnostics to carry the new fields. |
| .claude/skills/indexing-diagnostics/SKILL.md | Documents the new diagnostics fields and adds SQL/examples for computed hot-path triage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two robustness fixes around the new `beginComputePass` / `endComputePass` lifecycle: 1. **Stale `api` fallback.** During a cold dev start the host can briefly load a `base/card-api` build that predates the beginComputePass / endComputePass exports — vite is still optimizing dependencies or a transpile race serves a stale module. Without a guard this surfaced as `api.beginComputePass is not a function` errors during the very first reindex on a freshly-started stack, then went away once modules stabilized. Skip the pass when the methods aren't on `api` — `getter`'s `passComputeMemo === null` fast path still produces a correct serialized doc and searchDoc, just without per-row computedCalls / computedCacheHits diagnostics for those few cards. serializeMs + searchDocMs still surface either way. 2. **Close in `finally` on throw.** `endComputePass` now runs in a `finally` so a throw inside `serializeCard` or `searchDoc` still closes the pass — otherwise the module-global `passComputeMemo` in field-support.ts stays set and later off-pass `getter` calls would read stale memoized values across reactive cycles. Caught by Codex and Copilot bot review on PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2102efa to
77cefe7
Compare
…tract The dedup broke the `non-isolated formats render linked fields and those links appear in search doc` prerendering test (Realm Server Tests shard 1, 6). The two render.meta calls aren't duplicate work after all. Between them the fitted / embedded ancestor renders touch linksTo / linksToMany values from the embedded template; those reads mark the fields as "used" in the per-instance data bucket. The *second* renderMeta's queryableValue runs `getFields` with `usedLinksToFieldsOnly: true`, which now picks up those linked fields and includes their values in `searchDoc.owner.name`, `searchDoc.owners[*].name`, etc. Collapsing the two calls runs searchDoc *before* any fitted / embedded render and skips those fields entirely. Reverts only the render-runner change. The double-read fix in BaseDef.[queryableValue] and the pass-scoped memo are independent wins and stay — they're what produced the 63-66% reduction in computeVia invocations across compute-heavy cards. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…formance-improvements # Conflicts: # packages/host/app/routes/render/meta.ts
Reduces redundant
computeViainvocations during the per-card prerender by ~60–67% on the heaviest cards via two complementary fixes: the gist's "double-read" Phase 2 plus a pass-scoped compute memo. Driven by CS-11208 and the computed-performance gist.Background
The gist proposed a six-phase optimization plan for computed fields. Audited each phase against the current code:
BaseDef.[queryableValue](Phase 2) — for every contains / contains-many / links-to field in the search doc, we ranpeekAtField(which invokescomputeVia) and then re-readvalue[fieldName]through the descriptor (which invokescomputeViaagain). One traversal walked every computed twice. Landed.serializeCard + searchDoctogether touch the same(instance, fieldName)pair multiple times. A WeakMap-backed memo opened just for the synchronous traversal collapses those to a singlecomputeViainvocation each. Bounded to one sync traversal so it can't interact with Glimmer reactivity. Landed.render.metacalls collapsed into one — initially attempted; reverted after CI. The two calls aren't redundant: between them the fitted/embedded ancestor renders cause linksTo / linksToMany template reads to mark fields as "used" in the data bucket, which the second render.meta'squeryableValuethen includes in the search doc viausedLinksToFieldsOnly: true. Thenon-isolated formats render linked fields and those links appear in search doctest caught this. Filed as follow-up.Deferred from the gist: Phases 5 (shared aggregate rollups) and 6 (dependency-aware invalidation) — not worth complexity until we measure post-this-PR baselines.
Changes
packages/base/field-support.tsbeginComputePass()/endComputePass()open a synchronous per-instance memo (WeakMap<BaseDef, Map<string, any>>).getter()consults the memo when a pass is open; off-pass reads pay only oneif (passComputeMemo === null)branch.computedCalls+computedCacheHitsare incremented only inside the pass — production reads outsiderender.metaare unaffected.packages/base/card-api.gtsBaseDef.[queryableValue]reuses the value already peeked at the top of each iteration instead of re-readingvalue[fieldName]. For computed fields the re-read re-invokedcomputeVia— this is the gist's Phase 2 fix.beginComputePass,endComputePass,ComputePassSnapshotso the host route can consume them.packages/host/app/routes/render/meta.tsserializeCard+searchDoctraversal withbeginComputePass/endComputePass.PrerenderMetaDiagnostics { computedCalls, computedCacheHits, serializeMs, searchDocMs }on the PrerenderMeta response.host:computed-perflogger for local dev.try { ... } finally { endComputePass() }so a throw insideserializeCardorsearchDocalways closes the pass — without this, the module-global memo infield-support.tswould stay set and later off-passgettercalls would read stale memoized values across reactive cycles (raised by Codex + Copilot bot review).typeof api.beginComputePass === 'function'guard — during a cold dev boot the host can briefly load a stalebase/card-apibuild (vite still optimizing, or a transpile race). In that window the route skips the pass;getter's fast-path produces a correct doc + searchDoc, just without per-row compute counters for those few cards.packages/realm-server/prerender/render-settlement.tsdecorateRenderErrorsWithTimingsnow lifts the host's success-pathdiagnosticsblock offresponse.card(same way it already lifts error-path diagnostics). This is howcomputedCallsetc. reach the indexer'sboxel_index.timing_diagnosticscolumn.packages/runtime-common/index.tsPrerenderMetaDiagnosticsinterface; extendsPrerenderMetawith an optionaldiagnosticsfield.RenderTimeoutDiagnosticswith the same four fields so they ride alongside the existing server-observed timings inboxel_index.timing_diagnosticsanderror_doc.diagnostics..claude/skills/indexing-diagnostics/SKILL.mdcomputedCalls > 1000ANDsearchDocMs + serializeMs ≈ renderElapsedMs→ look at the card's computeds, not data loads or browser stalls).computedCallsso operators can find the compute-heavy outliers.Measurement methodology
Local stack against a compute-heavy fixture realm (an insurance-pricing workload with aggregate cards over a
linksToManypolicy collection — the aggregate card has 27 computeds, each linked card has 64). Triggered a single-realm reindex via the_grafana-reindexroute and readcomputedCalls/computedCacheHits/serializeMs/searchDocMsstraight offboxel_index.timing_diagnostics.Identifying card slugs are in CS-11208; the table below uses role labels.
Results
Across the compute-heavy slice the memo elides 63–66% of compute reads consistently — combined gain from the double-read fix + pass-memo. The gist's Phase 2 alone projected 10–40% on this card shape; with the pass-memo layered on, we hit the upper end of Phase 3's 50–80% range.
Hot-path overhead — direct microbenchmark
Concern was that adding any per-
gettercheck might tax the host UI's tight reactive-read loop where pass is closed. Measured directly in the running host app:A typical
computeViainvocation costs 1–100 µs. The new branch costs ~10,000× less than the work it gates and is well below the floor of anything visible in a Chrome flame chart.Follow-up: optimizing the render.meta double pass
The current two-call
renderMetaexists because the second call'squeryableValuedepends on linksTo / linksToMany field-usage state that the fitted/embedded renders between the calls populate. A clean win is still possible (e.g. expose card types via a cheaper synchronous endpoint, or run a single render.meta after all formats), but it needs its own design + test plan. Tracked in CS-11237.Deferred from the gist
compute.bxl.deps/ acomputeDepsannotation) — BXL-specific today; needs an additive opt-in for plain-JS computeds; plumbing throughgetFieldsis real work. Deferred until incremental edit numbers prove it necessary.Test plan
non-isolated formats render linked fields and those links appear in search docregression triggered the revert in commit 3)computedCallspopulated on every rowserializeMs + searchDocMsdoesn't regress wall-clock for compute-light cards (e.g. base realm cards)pnpm test --filter @cardstack/host) — pass-memo lifecycle must not interact with Glimmer's tracked entanglement🤖 Generated with Claude Code