Skip to content

perf(cost): cache the quota-week projection off the main thread - #3831

Open
giuseppebisemi wants to merge 1 commit into
steipete:mainfrom
giuseppebisemi:fix/quota-week-projection-main-thread
Open

giuseppebisemi wants to merge 1 commit into
steipete:mainfrom
giuseppebisemi:fix/quota-week-projection-main-thread

Conversation

@giuseppebisemi

@giuseppebisemi giuseppebisemi commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Fixes #3827.

The problem

Every Codex or Claude menu card build calls quotaWeekSummaries on the main thread, and that call rebuilds its per-day projection from scratch. The projection visits every exact slice. For each one it created a Calendar, read the date components, formatted a day key with String(format:), and asked the calendar for the hour interval.

My 30-day Claude history has about 54,000 slices. In a release build that pass takes 142 ms, and the card pays it again on every build.

What I changed

  1. quotaProjectionDays and quotaHourlyResidualSlices now use the day-key and hour-start memos that OpenCodexUsageAggregator already had. I moved the two structs next to CostUsageLocalDay so both callers share them. Nothing about them changed except the names.
  2. The snapshot memoizes its projection. The projection depends on the snapshot's immutable entries and the bucket time zone, so that is all the memo keys on. Reset observations, now, and the window boundaries are still evaluated on every call, which was the concern ClawSweeper raised about a timestamp-keyed cache. The memo is excluded from ==, and copies of a snapshot share it.
  3. UsageStore warms the projection from a utility task when it publishes or installs a snapshot, so the menu finds it ready instead of computing it while opening.

Numbers

Release build, 54,000 synthetic slices over 30 days plus hourly buckets, median of 10 runs, M-series Mac, macOS 26.7:

per call
main 142 ms
memos only 20.5 ms
this PR, first call on a snapshot 28 ms
this PR, later calls 2.5 ms

The weekly totals are identical in all four rows.

I also sampled the running app (sample <pid> 60 1) while switching tabs. On 0.63.0 the main thread spent about 1.9 s in quotaWeekSummaries, with single calls between 80 and 195 ms. With this branch it spent 219 ms across 35 calls, about 6 ms each, and the per-slice pass no longer shows up on the main thread at all.

A debug build hides most of this (216 ms to 192 ms with the memos), so measure in release. swift test -c release does not compile for me because of an unrelated key-path error in TestsLinux/PlatformGatingTests.swift; I benchmarked through a small external package that links CodexBarCore.

Tests

New: QuotaWeekProjectionScaleTests builds 54,000 slices across the 2025-10-26 fall-back day in Europe/Rome, checks each weekly total against a direct sum of the slices inside the window, and checks that a warmed snapshot and a cold one agree in a second time zone.

ProviderArchitectureGatekeeperTests only has line numbers re-anchored for UsageStore+TokenCost.swift, because the new lines shifted them. No entries were added or removed.

Commands run:

  • swift test --filter CostUsageQuota
  • swift test --filter InlineCostHistoryDashboardLabelTests
  • make check (0 violations)
  • make test (all shards pass)

The full suite ran before I rebased onto current main. After the rebase I re-ran the two filters above, ProviderArchitectureGatekeeperTests, the new test, and make check.

Sample output

Both captures are sample <pid> 60 1 on the same Mac (macOS 26.7, 25G229) while I opened the menu and switched between the Codex and Claude tabs about ten times. The lines below are every quotaWeekSummaries frame in the main-thread call graph, grouped by call-site offset. Each number is the sample count of one stack, and at a 1 ms interval a count reads as milliseconds.

I produced them with:

E=$(grep -n "Total number in stack" sample.txt | head -1 | cut -d: -f1)
head -n "$E" sample.txt | grep "quotaWeekSummaries(resetAt"

Before, release 0.63.0 (151):

Analysis of sampling CodexBar every 1 millisecond
Version:         0.63.0 (151)
OS Version:      macOS 26.7 (25G229)

CostUsageTokenSnapshot.quotaWeekSummaries(...)  (in CodexBar) + 2488
  stacks=16 total=1891 ms max=195 ms
  samples: 194 192 195 80 81 79 79 77 118 113 113 114 112 113 116 115
CostUsageTokenSnapshot.quotaWeekSummaries(...)  (in CodexBar) + 3060
  stacks=16 total=35 ms max=5 ms
  samples: 5 3 3 1 1 1 2 1 2 3 2 3 2 2 2 2

The + 2488 site is the call into quotaProjectionDays. One raw line from that capture, so you can see the shape:

194 CostUsageTokenSnapshot.quotaWeekSummaries(resetAt:windowMinutes:observedNextResets:observedResetInstants:resetObservations:weekCount:now:calendar:)  (in CodexBar) + 2488  [0x101ab5050]

After, this branch built with ./Scripts/compile_and_run.sh (release, 0.63.1 (152)):

Analysis of sampling CodexBar every 1 millisecond
Version:         0.63.1 (152)
OS Version:      macOS 26.7 (25G229)

CostUsageTokenSnapshot.quotaWeekSummaries(...)  (in CodexBar) + 2980
  stacks=35 total=219 ms max=21 ms
  samples: 19 9 9 3 8 4 3 8 9 2 2 17 17 21 2 2 9 4 4 1 1 11 10 2 3 5 4 7 5 6 2 2 3 1 4
CostUsageTokenSnapshot.quotaWeekSummaries(...)  (in CodexBar) + 2800
  stacks=7 total=8 ms max=2 ms
  samples: 1 2 1 1 1 1 1

The projection call site no longer appears on the main thread. The frames under + 2980 symbolicate with atos (against the unstripped build product) to projectQuotaWindow and projectQuotaTokens. That loop still runs per call because it depends on now and the reset boundaries.

Benchmark transcript (release, external package linking CodexBarCore, 54,000 slices, ten consecutive calls on one snapshot):

main:     ms: 156.3 142.8 141.2 141.0 139.7 142.7 144.3 142.0 139.9 140.5  median: 142.0
this PR:  ms: 28.4 2.5 2.5 2.5 2.4 4.2 4.4 2.5 2.4 2.4  median: 2.5
tokens (both): [110003958, 168000000, 168000000, 168000000]

ClawSweeper is right that warming is best effort. The utility task starts after publication, so a card built in that gap either computes the projection itself or waits on the memo's lock. That costs the 28 ms cold figure once per snapshot, against 142 ms on every build today.

Not in this PR

The first menu open is still heavier than I would like. The sample points at warmMergedSwitcherSiblingContent, which builds every sibling tab in one main-thread pass about 0.12 s after the menu opens (1.16 s in my sample, nearly all SwiftUI layout). That is a different mechanism from this issue, so I left it out. I have a small change that builds one tab per timer fire and can open it separately if you want it.

quotaWeekSummaries rebuilt its per-day projection on every menu card build.
The projection walks every exact slice and, per slice, created a Calendar,
read date components, formatted a day key with String(format:), and asked
the calendar for the hour interval. On a 30-day Claude history (~54k slices)
that is ~140 ms on the main thread each time a Codex or Claude card is built.

- Reuse the day-key and hour-start memos that OpenCodexUsageAggregator
  already had; they move next to CostUsageLocalDay so both callers share them.
- Memoize the projection inside the snapshot. It depends only on the
  snapshot's immutable entries and the bucket time zone, so reset
  observations, `now`, and window boundaries are still evaluated per call.
- Warm the projection from a utility task when UsageStore publishes or
  installs a snapshot, so the menu finds it ready.

Release build, 54,000 slices: 142 ms -> 28 ms cold, 2.5 ms warm, same output.

Refs steipete#3827
@clawsweeper

clawsweeper Bot commented Sep 21, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 21, 2026
@clawsweeper

clawsweeper Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 21, 2026, 9:10 AM ET / 13:10 UTC (Revision 2).

ClawSweeper review

What this changes

The PR caches weekly quota-history projections per snapshot and time zone, reuses calendar helpers, and warms the cache in a background task to reduce menu stalls.

Merge readiness

Ready for maintainer review

Keep open as a useful fix that current main still needs. The updated runtime excerpts satisfy the previous proof request, and the reviewed patch has no blocking correctness findings.

Priority: P2
Reviewed head: 6e9629f006510357389e2ef51d6af83a052ef26b

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused optimization with relevant real-app evidence, regression coverage, and no identified blocking defect.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (logs): The updated macOS app-sampling excerpts exercise quotaWeekSummaries during real provider-tab switching after rebuilding the branch and show reduced main-thread work. They resolve the prior request; synthetic benchmarks and regression tests provide supplemental evidence.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (logs): The updated macOS app-sampling excerpts exercise quotaWeekSummaries during real provider-tab switching after rebuilding the branch and show reduced main-thread work. They resolve the prior request; synthetic benchmarks and regression tests provide supplemental evidence.
Evidence reviewed 9 items Reviewed introduction: Read the complete six-file introduced diff between the pinned base and original head, plus original projection and calendar-helper implementations. The helper move preserves its existing behavior; the gatekeeper changes only re-anchor existing entries.
Current main still needs the optimization: Current main directly rebuilds quotaProjectionDays on each quotaWeekSummaries call. The menu consumer invokes this path when displaying quota-week history. No equivalent snapshot projection cache exists in the inspected implementation.
Cache correctness and concurrency: The cache belongs to an immutable snapshot, serializes access with NSLock, and keys projections by time zone. Both entrypoints normalize calendars to Gregorian-in-time-zone. Current time, reset evidence, window boundaries, and completeness decisions remain outside the cache; memo contents do not affect snapshot equality.
Findings None None.
Security None None.

How this fits together

CodexBar turns local usage history into weekly token and cost totals displayed in provider menu cards. This change caches the expensive history preparation while continuing to calculate live reset boundaries for each presentation.

flowchart TD
  A[Local usage history] --> B[Immutable usage snapshot]
  B --> C[Background cache warming]
  D[Selected time zone] --> C
  C --> E[Cached daily projection]
  B --> E
  E --> F[Calculate quota weeks]
  G[Current time and reset observations] --> F
  F --> H[Provider menu card]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +111/-62 (net +49); tests +75/-18 (net +57) Growth is justified by snapshot caching and focused scale coverage, with existing calendar helpers relocated.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3827
Summary: This PR is the explicit candidate fix for the linked menu projection performance report.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Reuse immutable history projections while keeping live quota boundaries fresh and calendar behavior unchanged.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes repeated synchronous projection work when quota-history cards are built, and contributor sampling demonstrates the associated stalls. This review did not execute a current-main reproduction.

Is this the best way to solve the issue?

Yes. Caching only immutable preparation avoids stale reset calculations, and reusing existing calendar helpers keeps the optimization narrow.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 7bb3dfe9697b.

Labels

Label justifications:

  • P2: Addresses observable provider-menu stalls with a bounded performance improvement.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The updated macOS app-sampling excerpts exercise quotaWeekSummaries during real provider-tab switching after rebuilding the branch and show reduced main-thread work. They resolve the prior request; synthetic benchmarks and regression tests provide supplemental evidence.
  • proof: sufficient: Contributor real behavior proof is sufficient. The updated macOS app-sampling excerpts exercise quotaWeekSummaries during real provider-tab switching after rebuilding the branch and show reduced main-thread work. They resolve the prior request; synthetic benchmarks and regression tests provide supplemental evidence.

Evidence

What I checked:

  • Reviewed introduction: Read the complete six-file introduced diff between the pinned base and original head, plus original projection and calendar-helper implementations. The helper move preserves its existing behavior; the gatekeeper changes only re-anchor existing entries. (6e9629f00651)
  • Current main still needs the optimization: Current main directly rebuilds quotaProjectionDays on each quotaWeekSummaries call. The menu consumer invokes this path when displaying quota-week history. No equivalent snapshot projection cache exists in the inspected implementation. (Sources/CodexBarCore/CostUsageModels+QuotaWindows.swift:51, 7bb3dfe9697b)
  • Cache correctness and concurrency: The cache belongs to an immutable snapshot, serializes access with NSLock, and keys projections by time zone. Both entrypoints normalize calendars to Gregorian-in-time-zone. Current time, reset evidence, window boundaries, and completeness decisions remain outside the cache; memo contents do not affect snapshot equality. (Sources/CodexBarCore/CostUsageModels+QuotaWindows.swift:775, 6e9629f00651)
  • Background warming scope: Publication and cache installation launch detached utility work using the selected bucket calendar. Warming is best effort: an immediate reader can still perform or wait for one cold calculation. This limits the performance guarantee but does not introduce a demonstrated regression. (Sources/CodexBar/UsageStore+TokenCost.swift:291, 6e9629f00651)
  • Updated real-app proof resolves prior request: The supplied body snapshot, sourceRevision fc07c954d8d609b5b3c8f1d166a50ed8072d4385d691562f89179468d73d25d0, contains before/after sample output from macOS 26.7 while opening the menu and switching Codex/Claude tabs. It identifies the freshly built branch app, reports 219 ms across 35 remaining projection frames versus 1,891 ms at the prior expensive call site, and reports that the per-slice preparation frame disappears from the main thread. The accompanying release benchmark reports 142 ms versus 2.5 ms median with identical totals. These excerpts address the sole prior rank-up request; embedded commands were not executed. (6e9629f00651)
  • Focused regression coverage: The added test checks 54,000 exact slices across Rome's fall-back transition against direct weekly sums, repeated calls, and a separate UTC calculation. Existing quota tests cover reset chronology, missing metadata, history boundaries, and partial totals. Contributor-reported checks include focused tests, make check, and a pre-rebase full suite; this read-only review ran no builds or tests. (Tests/CodexBarTests/QuotaWeekProjectionScaleTests.swift:32, 6e9629f00651)

Likely related people:

  • stabey: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-21T13:00:02.872Z sha 6e9629f :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

First provider tab switch stalls 100–200 ms: quotaWeekSummaries runs on the main thread with no cache

1 participant