perf: trim process startup, diagnostic dispatch, and foreground command latency - #5664
Closed
AdityaVG13 wants to merge 8 commits into
Closed
perf: trim process startup, diagnostic dispatch, and foreground command latency#5664AdityaVG13 wants to merge 8 commits into
AdityaVG13 wants to merge 8 commits into
Conversation
strace census on the doctor path showed 88.7% of syscall time in futex with 45 clone3 spawns: build_runtime() built a full multi-thread runtime (one worker per CPU, 16MiB stacks each) for a read-only diagnostic that uses none of it. Cap workers at 2 for Commands::Doctor only; interactive sessions and servers keep default sizing. Verified on spark-1672 (aarch64): clone3 45->27, futex calls 265->99, doctor wall 45.4->42.8ms, --help output byte-identical, --version floor unchanged.
bundled_models_dev_catalog() re-parsed the ~50KB snapshot at every call site: the client route path, provider picker, provider lake, and fleet identity each paid an independent serde parse. Return a &'static catalog backed by OnceLock — the asset is include_str! constant, so sharing the parsed form is immutability-safe. Doctor wall is unchanged (the init path only parses once either way); this removes redundant parses on multi-call paths.
…'s rustc spawns RustC::resolve() proved presence by executing 'rustc --version' and discarding stdout; doctor's rustc_version() then launched a second rustc process to read the same banner. Each launch loads libLLVM. probe_executable_capturing() now records the banner during the probe (OnceLock), and the diagnostics path consumes it after an available() check. Verified on spark-1672 (aarch64): execve(rustc) 2->1 per run, doctor wall 42.8->37.6ms (-12%, n=60), 'rust:' line byte-identical to the toolchain's own 'rustc --version', help output unchanged.
codewhale eval is a sequential offline tool-loop; it needs no async concurrency, so build its runtime with the same 2-worker cap doctor uses instead of one worker per CPU. Interleaved A/B on spark-1672 under load 9-11: eval wall -8/-10/-6 percent across three OLD/NEW batch pairs; thread spawns for the command drop from ~45 to 5. Interactive surfaces unchanged. style(tui): rustfmt the runtime builder chain
setup --status, sessions listing, and session diagnostics share the doctor dispatch shape: read-only, short-lived, no async concurrency need. Extend diagnostic_worker_count to cover them via a structured match mirroring telemetry_command_is_read_only. Interleaved A/B on spark-1672 for 'setup --status' (-17/-21/-16 percent across three batch pairs, NEW wins all three); interactive and mutating surfaces unchanged.
…ercent The persisted models.dev cache stores a ~5MB catalog body JSON-escaped inside a JSON envelope. maybe_load_persisted_cache() runs synchronously on the interactive boot path and parsed the body twice (envelope, then re-parse of the escaped body) plus a full-body string copy — a gdb mid-boot sample caught serde_json::visit_map inside models_dev_live during the largest silent window of startup, and strace showed a 17.7ms zero-syscall compute burst after config load. v2 format: one-line JSON metadata header followed by the verbatim catalog body. Loading does one small header parse, one body parse, zero body copies; v1 envelopes still load via fallback and every refresh now writes v2. Measured on spark-1672 (aarch64), identical binary, interleaved batches over scratch CODEWHALE_HOMEs differing only in cache format: time-to-first-frame median 44.0/52.7/48.6/51.7ms (v1) vs 13.9/14.1/13.9/14.4ms (v2).
apply_provider_model_cutlines called ApiProvider::parse per offering; parse scans every provider and its alias list with case-insensitive compares, and the live models.dev snapshot carries thousands of rows. Freeze-and-inspect at t+180ms of boot caught the main thread inside this loop. Resolve each distinct provider string once through a HashMap. Interleaved A/B on spark-1672: first-paint median drops ~2-6ms per run (195.7/196.9/192.9/197.2 -> 185.0/190.9/190.3/193.8, NEW wins 4/4). perf(tui): adaptive poll cadence for foreground shell completion Foreground bash runs go through execute_foreground_via_background, whose wait loop polled child status on a fixed 100ms tick. A command that finished in 2ms was only noticed at the next tick, so every fast foreground call (ls, grep, wc, echo — the bulk of agent traffic) carried a ~100ms floor: a simulated 8-tool turn spent ~400ms of its ~526ms wall in poll quantization (measured over serve --mcp with the real registry). Replace the fixed tick in all three wait loops (foreground completion, wait-many, delta waiter) with an adaptive cadence: first sleep 10ms, then double to a 100ms cap. Instant commands are now detected within ~10ms; long-running commands reach the old cap after one doubling step, so their overhead is unchanged. Revert "perf(tui): adaptive poll cadence for foreground shell completion" This reverts commit 1a2f42c9f0b53d9b83ed50db4d4d7bd66df7977e.
Foreground bash runs go through execute_foreground_via_background, whose wait loop polled child status on a fixed 100ms tick. A command that finished in 2ms was only noticed at the next tick, so every fast foreground call (ls, grep, wc, echo — the bulk of agent traffic) carried a ~100ms floor: a simulated 8-tool turn spent ~400ms of its ~526ms wall in poll quantization (measured over serve --mcp with the real registry). Replace the fixed tick in all three wait loops (foreground completion, wait-many, delta waiter) with an adaptive cadence: first sleep 10ms, then double to a 100ms cap. Instant commands are now detected within ~10ms; long-running commands reach the old cap after one doubling step, so their overhead is unchanged. Measured over real serve --mcp (n=150, binaries aside): bash `true`: p50 13.6 both | p95 16.2→13.9 | p99 21.5→18.6 | p100 35.9→21.6 bash grep: p99-p100 flat ~13.8-14.1 both file_read: p100 0.2 both (min/p50/p99 all 0.1) tools/list: p100 0.2-0.7 both Tail-wall win: p95 -14%, p99 -13%, p100 -40% with p50 unchanged. An earlier A/B that compared only medians missed the win and was reverted; the tail distribution above is what justified re-landing it. Interleaved A/B on aarch64 Linux, same host, same window.
Owner
|
Thank you, Aditya. I verified all eight patches in this PR are already on Landed commits: I’m closing this duplicate PR because the contribution is already shipped on |
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.
perf: trim process startup, diagnostic dispatch, and foreground command latency
Three clusters of changes, each targeting waste found by profiling rather than guesswork: diagnostic subcommands built a 45-thread tokio runtime they never used; the models.dev catalog was parsed repeatedly per process and twice per byte on the boot path; and every foreground shell call carried a fixed 100 ms poll tick no matter how fast the command finished. All measurements below come from interleaved same-window batch pairs on one aarch64 Linux machine, with attribution from strace censuses and mid-boot sampling.
doctorwall, quiet host, n=60evalwall, under load, 3 interleaved pairssetup --statuswall, under load, 3 pairstrue, tail (n=150, real MCP serve)Runtime sizing for read-only diagnostics
A strace census on the doctor path showed 88.7% of syscall time in futex behind 45 thread spawns:
build_runtime()built one worker per CPU with 16 MiB stacks each for read-only commands that use none of it.doctor,eval,setup --status, and the sessions/diagnostics family now cap workers at 2 through a structured match mirroringtelemetry_command_is_read_only. Interactive sessions and servers keep default sizing.Parse the bundled catalog once
bundled_models_dev_catalog()re-parsed the ~50 KB snapshot at every call site: client routing, provider picker, provider lake, and fleet identity each paid an independent serde parse. It now returns a&'staticcatalog backed byOnceLock. The asset is aninclude_str!constant, so sharing the parsed form is immutability-safe. One existing assertion is adapted to the new signature and the build-time deep-equality guard keeps its exact semantics.Capture the rustc banner during the probe
RustC::resolve()proved presence by runningrustc --versionand discarding stdout; the diagnostics path then launched a second rustc process to read the same banner, and each launch loads libLLVM. The probe now records the banner behind aOnceLockand diagnostics consume it after an availability check. The reportedrust:line is byte-identical to the toolchain's ownrustc --versionoutput.Single-parse disk cache, v2 format
The persisted catalog stored a ~5 MB body JSON-escaped inside a JSON envelope, and the synchronous boot path parsed it twice plus a full-body string copy. Mid-boot sampling caught
serde_json::visit_mapinside the largest silent window of startup, alongside a 17.7 ms zero-syscall compute burst after config load. v2 is a one-line JSON metadata header followed by the verbatim body: one small header parse, one body parse, zero body copies. Every refresh writes v2 and v1 envelopes still load through a fallback, so the first run after upgrade migrates the cache with no manual step.Memoize provider resolution in the cutline pass
apply_provider_model_cutlinescalledApiProvider::parseper offering. Parse scans every provider and its alias list with case-insensitive compares, and the live snapshot carries thousands of rows. Each distinct provider string is now resolved once through a map.Adaptive poll cadence for foreground shell completion
The foreground wait loop polled child status on a fixed 100 ms tick, so a command finishing in 2 ms was only noticed at the next tick. A simulated 8-tool turn spent ~400 ms of its ~526 ms wall in poll quantization. The tick now starts at 10 ms and doubles to the old 100 ms cap in all three wait loops, so instant commands are detected about 10x sooner while long-running commands reach the same steady state after one doubling step. A first A/B that compared only medians missed the win and was reverted; the tail distribution above is what justified re-landing it, so the revert and reapply are squashed away in this branch's history.
Method
Latency claims come from interleaved OLD/NEW batch pairs on the same host in the same window, n=60 on a quiet host for doctor and three batch pairs under synthetic load where the host was busy. Goldens:
--helpand--versionoutput byte-identical, therust:line matches the toolchain banner, eval behavior identical across a 6-step run.Testing
cargo fmtclean. Config crate: 603 passed. TUI modules touched by this series (dependencies,catalog,models_dev_live,provider_lake,shell): 270 passed at the branch tip.Reviewer note, unrelated to this series
runtime_api::tests::reload_config_preserves_profile_selected_named_custom_routeoverflows its stack in the full lib-test run. It reproduces identically on a pristine checkout ofmainin a throwaway worktree, so it is pre-existing and untouched here.