perf(code-index): stop the readiness waits competing with the rebuild - #1580
Draft
ScriptedAlchemy wants to merge 6 commits into
Draft
ScriptedAlchemy wants to merge 6 commits into
ScriptedAlchemy wants to merge 6 commits into
Conversation
A retryable graph activation used to erase the prepared serving candidate, and an unfinished clone-fingerprint successor withheld the same seat after exact and lexical owners were ready. Keep the candidate in both cases so search can move off the predecessor while graph retries. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
The graph-rebuild receipt timed out for 90s against a generation that was already current: status reported `current` on the expected revision with the advertised generation matching what search served, and the seat never moved for the whole wait. `limit: 3` still rendered 18084 characters against the 15000-character response frame, so MCP replaced the body with a retrieval handle and moved `results` and `code_generation` inside `preview`. Every predicate read them as absent and the wait spun to its deadline. One candidate carries several KiB of ranking provenance, so the page has to be smaller than a guess at how many results fit. Ask for one result, and refuse a truncated envelope outright rather than reading it as a warming generation, so the next frame overflow reports itself instead of presenting as a deadline. Pace both waits as well: each `tracedecay_status` call runs the census ready-probe, a freshness read, and branch diagnostics, and a `yield_now` spin re-entered that path thousands of times a second on the runtime running the reconcile it waits for. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
`GitMetadataFingerprintV1::capture` is the tier-1 staleness signal sampled on every query admission, and its own contract calls that cost fixed and cheap. It was neither: resolving the git-dir and common-dir through a fresh `gix::open` cost 72.7us of the 75.2us per capture, and runtime-core already owns a revalidating topology memo that answers the same question in 2.0us. Search runs two to three captures per call. Measured on a one-ref fixture repository, perf profile, 2000 warm iterations: capture 75.2us -> 10.0us. The memo is asked only for a checkout carrying `<root>/.git`, which is both where an open at exactly this root resolves through and where a discovery started at this root stops, so it returns the same two paths. A bare control directory or a path that is not a checkout root still opens directly, because discovery would walk past it to an ancestor whose git metadata does not describe this project. The memo canonicalizes both paths; the fingerprint samples file metadata and contents, so its value and its persisted signature are unchanged. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
The production composition's publication wait re-ran its readiness probe every 10ms for up to 20 seconds. That probe canonicalizes the root, takes the scheduler registry's mounted mutex several times, offloads a Git-metadata freshness capture to the blocking pool, and emits a decline event — spent on the same cores as the reconcile it waits for, and leaving a pending arrival the worker yields its graph prepare to. The registry already publishes the edge this wants: the serving watch signals every seat install and every source revalidation that keeps an unchanged generation seated. Drive the wait from it, keeping a 100ms floor for the terminal answers that install no seat — a route that has not mounted yet, and a verified source that publishes no generation at all. Measured on the graph-rebuild transport journey, perf profile: publication waits 55.1s -> 50.1s and 15.4s -> 14.5s, suite 164.7s -> 155.3s. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
Two suites reached the same wall — an MCP answer over the response frame
arrives as `{"truncated": true, "handle": …, "preview": …}`, where `preview`
is a string and `results` / `code_generation` are absent at the top level —
and only one of them handled it. The graph-rebuild receipt read that
envelope as a warming generation and spent its whole 90s deadline on a
generation that was already current; `limit: 20 -> 3` was a guess at how
many candidates fit, and 3 still rendered 18084 characters against 15000.
Lift the daemon suite's retrieve-paging resolver into the shared test
surface and read every tool answer through it, so the page size stops being
load-bearing: it is a property of how much ranking provenance a candidate
carries, not something a journey should track. Both suites' duplicate
`tool` / `tool_payload` helpers go with it.
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Contributor
Performance Comparison
|
Owner
Author
|
#1882 ports |
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
2 participants
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.
Investigation of the graph-rebuild receipt's 90s timeout, focused on the status-polling wait. Stacked on
cursor/seat-through-retryable-activation-f5c7(#1562); reviewingmaster...will show that branch's commits too.The 90s timeout was not waiting on anything
Reproduced locally (perf profile, 4 cores). At the moment the deadline fired:
code_index_freshness.statuscurrentcoverage/staleness_statecomplete/freshsource_reference/source_revisionrefs/heads/main/ the expected refreshed revisionlatest_generation_idgeneration.v1.51e0facb.00000002.3c00fba0…code_index_freshness_warningretrieval_serving.seated_generation_age_secondsEvery conjunct on the status side held, on the right revision, for the whole 90 seconds. The search response carried that same
code_generation— insidepreview, becauselimit: 3rendered 18 084 characters against the 15 000-character response frame (tracedecay-mcp-catalog), so MCP replaced the body with a retrieval handle:resultsandcode_generationare absent at the top level. The predicate read them as missing, could not distinguish that from a warming generation, and spun to its deadline.seated_generation_age_seconds ≈ 89is exactly that: the seat was installed one second in and never moved — the seat fix in #1562 worked, and the receipt could not see it.What blows the frame is ~5 KiB of ranking provenance per candidate, not result bodies, so
limit: 20 → 3in 7bdc33d was a guess at how many candidates fit and landed short.Changes
test(tracedecay)—daemon_suite/git_watch_test.rshad already hit this wall and solved it properly, by paging the stored original back throughtracedecay_retrieve. That resolver moves to the sharedtests/common/surface and both suites read every tool answer through it, so page size stops being load-bearing. Both suites' duplicatetool/tool_payloadhelpers go with it (−113/+16 lines).test(transport)— both waits paced at 25 ms instead ofyield_now. The spin re-enteredtracedecay_statusthousands of times a second, and each call runs the generation census ready-probe, a scheduler freshness read, and branch diagnostics — 1–3 Git opens, 2 blocking-pool offloads, 2–6 registry mutex acquisitions — on the runtime running the reconcile it waits for.perf(code-index)—GitMetadataFingerprintV1::capturere-derived the git-dir and common-dir through a freshgix::openon every sample, against a doc comment calling that cost fixed and cheap. runtime-core already owns a revalidating topology memo for the same question. Capture 75.2 µs → 10.0 µs; this is the tier-1 ladder on every query admission, and search runs 2–3 captures per call.perf(daemon)— the production composition's publication wait re-ran that probe every 10 ms for up to 20 s, leaving a pending arrival the worker yields its graph prepare to. The registry already publishes the edge it wants (subscribe_serving_generation_changes), so the wait parks on it, with a 100 ms floor for the terminal answers that install no seat.Measured
graph_rebuild_status_test::background_refresh_and_reopen_report_only_servable_generations, perf profile, 4 cores:Fingerprint, one-ref fixture, 2000 warm iterations: capture 75.2 µs → 10.0 µs;
gix::open72.7 µs; topology memo 2.0 µs.Publication waits with the event-driven wait: 0.45 s initial, 50.1 s reopen (from 55.1 s), 14.5 s unchanged reopen (from 15.4 s).
No timeout was raised. The 90 s
RECEIPT_TIMEOUTand the harness's 20 s budget are untouched; the wide budget above was investigation scaffolding and is not in the diff.What this does not fix
With the waits corrected, the journey's next binding constraint is the publication cost itself, and it is not a polling problem:
Indexing/PartialRefreshInProgress,clone_index: "the sealed lexical artifact is still building"— progressing, just whole-corpus.That belongs to the rebuild-cost lane and the fix is not a wider budget. Until it lands this test still fails — but at a truthful, self-describing point rather than a 90 s deadline on an already-current generation.
Also found, not changed here
daemon_suite/git_watch_test.rs:238andmcp_suite/.../graph_readiness.rs:42; both comparecode_generationacross two separate tool calls.tracedecay_statuscomputes the generation census unconditionally, including foradmission_only, andbuild_branch_diagnosticsresolves the live branch through a Git open plus abranch-metaread and several path stats on every call — none of it gated by theinclude_*flags.serving_generation_after_activation_failure(added in b80dd58, fix(code-index): seat text through retryable graph activation #1562) is dead outsidecfg(test)and warns on every build.Verification
daemon_suite::git_watch_test— 5 passed, confirming the shared resolver on the suite that owned it.code_index_scheduler::identity::tests— 5 passed, including both fingerprint change-detection tests and the in-place loose-ref rewrite.code_index_scheduler::*— 8 failures, all pre-existing on fix(code-index): seat text through retryable graph activation #1562: 7 of the 8 fail identically with this branch'sidentity.rsreverted to b80dd58, and the eighth (concurrent_query_admissions_claim_one_pending_wake_before_worker_coalescing) is flaky on base at 3/8 versus 1/5 here.cargo fmt,cargo check --tests, commitlint clean.