Improve CFG traversal - #160193
Conversation
|
@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.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (0d785d5): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf 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 -3.5%, secondary -1.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -7.0%, secondary -6.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 497.309s -> 488.837s (-1.70%) |
|
Great results on Also -12% walltime on |
|
Wow great results! |
`iterate_to_fixpoint` uses a FIFO worklist. It is seeded in a good order (reverse postorder for forward analyses, postorder for backward analyses). But when back edges are present, re-dirtied blocks are pushed to the back of the queue. Successor blocks of these re-dirtied blocks are then analyzed even though we know the input states may change later on. This commit switches to a new min-rank algorithm that always processes the earliest dirty block in dataflow order. On most code this makes little difference, but cranelift-codegen has one enormous function with over 18,000 basic blocks. The old algorithm required 1.5 million calls to `apply_effects_in_block` to reach a fixpoint for `EverInitializedPlaces`. The new algorithm requires 90,000. This results in a 17% reduction in instruction counts for a `Check Full` build. Note: the new algorithm also avoids doing anything for unreachable blocks in backward analyses. (The old algorithm computed states but never used them meaningfully.)
It's no longer used.
|
The improvement is entirely from After this PR, the 1508264 drops to 89809 and everything else is the same or barely changes. (Actually, the I also have a draft PR at #160033 to improve |
df3dea1 to
ac49776
Compare
|
I have updated the code to address the review comments. |
|
@bors r+ |
|
-8.5s in the bootstrap results is large enough that it might be a real effect and not just random fluctuations. Especially given that it all mostly from rustc_parse dropping from 19.8s to 11.6s. |
This comment has been minimized.
This comment has been minimized.
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 6c04025 (parent) -> cb9d1b0 (this PR) Test differencesShow 6 test diffs6 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard cb9d1b0640549f1b041aae430dc413ce93f8c204 --output-dir test-dashboardAnd 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 (cb9d1b0): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@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 -0.4%, secondary 0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -5.8%, secondary 0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 491.569s -> 490.618s (-0.19%) |
|
perf triage:
@rustbot label: +perf-regression-triaged |
View all comments
This PR implements a new CFG traversal algorithm in
iterate_to_fixpointthat drastically speeds up a pathological case incranelift-codegen.r? @cjgillot