-
Notifications
You must be signed in to change notification settings - Fork 13.7k
cleanup and cache proof tree building #145951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
095c024
to
db6498a
Compare
This comment has been minimized.
This comment has been minimized.
db6498a
to
158414f
Compare
This comment has been minimized.
This comment has been minimized.
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cleanup proof tree building + prrrrf
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (7f35a84): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.5%, secondary 1.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.7%, secondary -2.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 468.353s -> 465.542s (-0.60%) |
e6f9e26
to
b5f4d62
Compare
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
b5f4d62
to
936c027
Compare
936c027
to
0edb22c
Compare
@bors r+ rollup=never |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing f6df223 (parent) -> a2c8b0b (this PR) Test differencesShow 2 test diffs2 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard a2c8b0b92c14b02f0b3f96a0d5296f1090dc286b --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (a2c8b0b): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -1.5%, secondary 3.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.1%, secondary -2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 467.107s -> 465.026s (-0.45%) |
There's some cruft left over from when we had deep proof trees. We never encounter overflow when evaluating proof trees. Even if the recursion limit is
0
, we still only hit the overflow limit when evaluating nested goals of the root. The root goal simply inherits theroot_depth
of theSearchGraph
.Split
evaluate_root_goal_for_proof_tree
from the rest of the trait solver. This enables us to simplify the implementation ofevaluate_goal_raw
and theProofTreeBuilder
as we no longer need to manually track the state of the builder and can instead use separate types for that. It does require making a few internal methods into associated functions taking adelegate
and aspan
instead of theEvalCtxt
itself.I've also split
SearchGraph::evaluate_goal
andSearchGraph::evaluate_root_goal_for_proof_tree
for the same reason. Both functions don't actually share too much code, so by splitting them each version gets significantly easier to read.Add a
query evaluate_root_goal_for_proof_tree_raw
to cache proof tree building. This requires arena allocatinginspect::Probe
. I've added a new type aliasI::ProbeRef
for this. We may need to adapt this for rust-analyzer? It would definitely be easy to remove theCopy
bound here 🤔