perf(storage): reuse Usage range statistics across filter changes - #5517
Open
liuxiaocs7 wants to merge 1 commit into
Open
liuxiaocs7 wants to merge 1 commit into
liuxiaocs7 wants to merge 1 commit into
Conversation
Fixes apache#5516 Refs apache#5038 Generated-by: Codex
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changing only Usage activity search/status currently repeats complete range statistics. Keep one statistics projection in the existing Storage reader, keyed by its lifecycle-aware Usage revision and exact
from/to. Reuse summary, provider/model/tool groups, pricing and provenance while reading the revision, activity count and page in one transaction. Publish cache misses after the outermost commit; clone every response before Host identity projection/freezing.The deterministic facade regression drops repeated aggregate statements from 5 to 0. With 50,000 tool records, a search edit falls from 245.577 to 143.732 ms median on Node and 260.273 to 160.015 ms on Electron. This branch starts directly from main
0117d76c5688475e7467ee6db83057bf65edbabd; it complements #5514's pagination/count work and #5410's frontend work.Fixes #5516
Refs #5038
Verification
NODE_OPTIONS=--disable-warning=ExperimentalWarning.e8960e1d59ae2ac223111ef0bab3f87a7559ab98. These do not replace independent human review.npm testwas attempted and is not green. Storage's child-stderr assertion rejects Node's SQLite experimental warning, and a Host implementation-child-patch case times out; both reproduce in the main checkout. Three other Host lifecycle failures and one Eval deadline failure passed focused reruns. Eval's Python tests passed separately on Python 3.12 (75 passed, 12 skipped); the default macOS Python 3.9 cannot import the existing union annotations. Baseline comparisons reuse installed third-party dependencies and source-identical dependency build artifacts.Performance evidence
Cache behavior, reproducible measurements and integration notes
Reproduce
Build Core and Storage at the baseline and this PR commit, then run from this checkout:
On macOS, also exercise the shipped SQLite runtime:
Both versions call the real
readUsageScreenfacade against the same persisted temporary database with separate reader connections. Each case alternates before/after order in one process, uses 3 warmups and 15 samples, and checks complete result equality excluding the opaque revision. Untimed preparation loads the original range before a timed search/status change; changing the range or committing a real tool update measures misses. Fixture writes, equality assertions and SQL probes are outside timing. The harness saves raw samples, median/p95, environment, revisions and executed SQL.Fixtures contain 1k/10k/50k tool records or 50k mixed canonical/legacy/tool records, with bounded groups (20 models, 10 connections, 8 tools). These are synthetic local facade measurements; they exclude Host repair, wire serialization, IPC/network and rendering. No cold OS cache, physical row-visit or fixed-byte cache-size claim is made.
Results
Measured on 2026-09-20 against main
0117d76c5688475e7467ee6db83057bf65edbabdand this PR commite8960e1d59ae2ac223111ef0bab3f87a7559ab98, with a clean tracked working tree. Machine: Apple M4 Pro, macOS/arm64, 24 GiB RAM. Node v24.14.0 / SQLite 3.51.2; Electron 43.4.1 (Node v24.18.1) / SQLite 3.53.1. Each cell is median / p95, with nearest-rank p95 over 15 samples. These local results are not production latency guarantees.All search SQL probes report 5 aggregate statements before, 0 after: the model summary, three breakdown queries and unreadable-checkpoint SUM are skipped on a hit, along with pricing/provenance reads. This counts statements, not physical row visits. Both range-boundary and data-revision changes still recompute statistics; their timings show the remaining miss cost. Miss tail latency is variable: the Electron mixed-source revision-change p95 is 377.019 ms before and 472.248 ms after in this 15-sample run, so these results do not establish a miss-tail improvement.
Correctness and limits
The cache retains one count-bounded complete statistics projection, not an activity page or history array. Search and status do not participate in the statistics key. A range/revision miss still runs the original SQL; data/pricing/repair changes and reader close/reopen invalidate reuse. Storage group/pricing capacity failures are not cached. Public-facade tests cover rollback with counter reuse, response mutation/freezing, inclusive/empty ranges, corrections/deletion, pricing, projection repair, capacity recovery, concurrent WAL commits and separate roots. A real Host test verifies frozen replies and replacement-coordinator revisions.
Continuous Usage writes change the global revision and may prevent cache hits. First-range aggregation, exact activity counts and substring searches retain their range-sized costs; this PR leaves main's activity SQL intact. The cache is count-bounded rather than separately byte-bounded, and existing Host response-size checks still apply.
Composition with other Usage PRs
#5514 at
9bdfd1f51398f86edb2e2511c1d8ba1f4009fa82and this PR each build independently from main. A trial merge has one conflicted file,usage-screen.ts, because #5514 reuses totals where this PR extracts the statistics. The resolution keeps this cache, calculates unfiltered activity total asstatistics.summary.totalRequests + sum(statistics.byTool.calls), and retains #5514's bounded page queries and zero-count shortcut. That combined implementation passed 40/40 Usage tests on both Node and Electron in a disposable checkout. No PR1 changes are included here.#5410 at
a09e1b7625b8427425a962dd98a1a9cbc0755de8applies cleanly to this branch and the PR1/PR2 trial merge. That is a patch-compatibility check, not a test run of the three-PR combination.AI use
Select exactly one:
Tool(s) and scope: Codex implemented the reader cache, regression tests and benchmark, drafted the issue/PR evidence, and performed automated reviews. The commit includes
Generated-by: Codex; retain the trailer when squashing.Checklist
Usage-specific checks pass; the full local test gate has the baseline/environment failures listed above.
Does this PR entail a change in behavior?