Repository navigation
Conversation
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.
The repeated ~5–6% regressions in
repeated_suppressing_context_managersand its interleaved-calls variant are reproducible compiled-code differences. This temporary workflow replays the existing #28712 and main benchmark binaries on three fresh Depot hosts, measuring one base/head pair on each host while preserving the original micro-m-z shard order.Each replay verifies the executable hash, uses the same executable path, and saves raw instruction, cycle-estimation, and simulated-cache profiles. CPU details, libc versions, and library hashes allow comparisons within and across hosts. Experimental profiles are uploaded as GitHub artifacts without publishing them to CodSpeed.
This is a test workflow, not a proposed permanent CI change. Its fixed input artifacts expire on September 23.
Findings
Controlled replays of the original #28712 and #28771 binaries reproduce the effect in raw instruction counts: approximately +4.47% for the plain benchmark and +5.45% for the interleaved variant. The number of query lookups and actual query executions is unchanged. Repeating the identical baseline locally changes the instruction counts by less than 0.000008%.
The hot
std::thread::LocalKey::withspecialization containing Salsa's attachment and suppression-query lookup is inlined intoreachability::analyze_singlein the fast binaries and remains out of line in the regressed binaries. The main path executes 41 additional instructions per lookup, repeated about 1.245 million times in the plain benchmark, accounting for approximately 93% of its additional instructions. Both unrelated regressed PRs have identical normalizedanalyze_singleassembly; passing #28769 retains the inlined form.Debug information and object boundaries show a matching codegen-unit pattern: the caller and TLS-wrapper group share a unit in both baselines and passing #28769, but occupy different units in both regressed binaries. Rust's partition merging depends on crate-wide size and inline-item overlap, so unrelated edits can change their placement. This strongly implicates partition-sensitive inlining, although the precise local ThinLTO import or inline-cost decision has not been isolated. See Rust's CGU merging and the unannotated
LocalKey::with.The native diagnostic run completed successfully on all three hosts. Each host ran both original #28712 binaries, with these increases in raw costs:
All hosts report AMD EPYC 9R45, the same CPU flags and microcode, Linux 6.17.0-1019-aws, glibc 2.39-0ubuntu8.9, and identical libc and loader hashes. Their CodSpeed runner and Valgrind versions also match. Across all 26 benchmarks in this run, the identical baseline's maximum instruction-count spread is 0.150%; its maximum throughput-cycle spread is 0.736%. These are raw event counts, not exact bounds on CodSpeed's combined score; simulated cache-miss counts can vary more. Available historical logs also show matching runner, kernel, compiler, and measurement-tool versions, but did not record enough CPU/library metadata to reconstruct every original host.
The benchmark
profilingprofile uses local ThinLTO, while shipped releases use fat LTO. These results therefore establish a benchmark-build regression; they do not establish equivalent shipped-release impact. A targeted codegen-unit or inner-wrapper inlining experiment would be the next step toward stabilizing the benchmark builds. No mitigation is included here.Test plan
The experiment covers identical-binary reproducibility across fresh runners and the reported base/head difference within each runner. All three native comparisons completed successfully, measuring 26 benchmarks per binary. Workflow formatting, schema, security, and shell checks validate the diagnostic harness; all applicable PR checks passed.