Use DenseBit for drop_live_at in liveness tracing - #162488
Conversation
|
Cool :) r=me with comment added |
|
@bors rollup=never |
94f3a57 to
855183d
Compare
|
Cool, added a comment. Thank you! @bors r=jackh726 And for good measure: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Use `DenseBit` for `drop_live_at` in liveness tracing
This comment has been minimized.
This comment has been minimized.
855183d to
1b1b92e
Compare
|
This pull request was unapproved. |
This comment has been minimized.
This comment has been minimized.
Use `DenseBit` for `drop_live_at` in liveness tracing
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (57ac888): 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 0.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.6%, secondary -1.2%)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: 478.441s -> 476.44s (-0.42%) |
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 0d31508 (parent) -> 1edd55d (this PR) Test differencesShow 3 test diffs3 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 1edd55dcfcd573872c727fa3e086369a71661ee0 --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 (1edd55d): 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.1%, secondary 1.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.7%, secondary 0.5%)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: 480.949s -> 481.679s (0.15%) |
I noticed that
drop_live_atonly ever gets point, and never range, inserts, so I switched it over toDenseBitSet. This provides some nice wins onserde.However, the
add_drop_live_facts_forfunction expects to receive anIntervalSet. I first tried to switch the type ofLiveRegions::AtPointsfromSparseIntervalMatrixtoSparseBitMatrix, but that had regressions.So in this PR, I simply reconstruct an
IntervalSetfrom aDenseBitSet. Not super pretty, but perf. looks reasonable.Let me know if this makes sense or not! :)
r? jackh726