Skip to content

Cache external repository resolution and parallelize query decode (#457) - #460

Merged
tinder-maxwellelliott merged 6 commits into
Tinder:masterfrom
maxwellE:claude/457-external-repo-and-decode-perf
Aug 15, 2026
Merged

Cache external repository resolution and parallelize query decode (#457)#460
tinder-maxwellelliott merged 6 commits into
Tinder:masterfrom
maxwellE:claude/457-external-repo-and-decode-perf

Conversation

@tinder-maxwellelliott

Copy link
Copy Markdown
Collaborator

Applies the two fixes proposed in #457, validated independently with the performance harness from #459 (baseline binary in the reference slot, patched binary in the candidate slot, so the gate's output-parity check doubles as proof the fixes do not change emitted hashes).

Cache external repository resolution (src/hash.rs)

ExternalRepoResolver::resolve had no memoization and guessed only external/<repo> and external/<repo>+, which both miss under Bzlmod where the directory carries the canonical repository name (e.g. rules_req_compile++requirements+pip_deps). Every miss shelled out to bazel query @<repo>//... --output location — once per external source file, serialized on the Bazel server lock — and that fallback ran a bare bazel without the configured startup options, so under --bazelStartupOptions=--output_base=... it queried a different output base than the one it resolved paths against.

This change memoizes resolution per apparent repository name, answers canonical names from a single bazel mod dump_repo_mapping invocation, keeps the bazel query fallback for WORKSPACE-only builds, and gives that fallback the startup options the resolver was constructed with.

Measured on a hermetic fixture with 400 @pip_deps// source files living only under the canonical directory (replay bazel shim logging every invocation):

before after
nested query ... --output location invocations 400 (one per file) 0
mod dump_repo_mapping invocations 0 1
fallback queries carrying --output_base 0 of 400 n/a
wall time (free stub bazel) 0.358 s 0.033 s
wall time (50 ms per nested invocation) 5.61 s 0.034 s
emitted hashes identical

The invocation counts reproduce #457's mechanism exactly (709 nested queries → 0 in the reporter's repository); with real per-invocation Bazel cost the before-time scales linearly with external source files, consistent with the reported 300 s → 2.4 s.

Decode Bazel query output in parallel (src/bazel.rs)

decode_target_stream decoded one message at a time off a default 8 KiB BufReader, entirely on one thread. This change reads length-delimited messages into bounded batches (8192 messages / 32 MB), decodes each batch across the Rayon pool, walks the decoded batch in stream order (transform stays sequential because callers accumulate into shared state), and raises the reader buffer to 1 MiB.

Measured with the #459 gate (4 CPUs, 7 interleaved rounds, ~48k-target graph): 1.04–1.08x end-to-end on generate-hashes with hashes identical in every round, peak RSS unchanged within 4% (115 MB → 119 MB), and no movement outside noise on the diff workloads the change does not touch. #457 reports 1.4x on the query-parse phase alone for a 1.05 GB / 576k-target stream.

Tests

Both patches ship unit tests (repo-mapping resolution and caching, fallback startup options, mapping parser, varint edge cases, order preservation across batch boundaries, truncated-input rejection). Full lib suite: 81 passed.

Credit to @BarrettStephen for the diagnosis and both patches in #457.

Closes #457

claude added 6 commits August 14, 2026 23:42
Memoize ExternalRepoResolver::resolve per apparent repository name and
resolve canonical directory names from one 'bazel mod dump_repo_mapping'
invocation, keeping the 'bazel query' fallback for WORKSPACE-only builds
and giving that fallback the startup options the resolver was
constructed with. Patch proposed inline in Tinder#457;
applied here to validate it. Four pre-existing test constructor sites
gained '..Default::default()' to match the widened struct.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JKHtwyVvPowHmDhiwqKJ5g
Read length-delimited messages into bounded batches (8192 messages,
32MB), decode each batch across the Rayon pool while keeping transform
sequential and in stream order, and raise the reader buffer to 1MiB.
Patch proposed inline in Tinder#457; applied here to
validate it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JKHtwyVvPowHmDhiwqKJ5g
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JKHtwyVvPowHmDhiwqKJ5g
Rust candidate E2E failed on the merge commit with no changes to any
code, test, or dependency file relative to the previous green run;
retriggering.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JKHtwyVvPowHmDhiwqKJ5g
Rust candidate (Bazel 9.x) failed //tools:perf_gate_test's
test_passes_when_rust_is_consistently_faster, a sleep-margin timing
test that loses rounds under runner load; reproduced the flake locally
under CPU contention with no tree changes. Retriggering.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JKHtwyVvPowHmDhiwqKJ5g
@tinder-maxwellelliott
tinder-maxwellelliott merged commit 19e0164 into Tinder:master Aug 15, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rust bazel-diff feedback

2 participants