-
Notifications
You must be signed in to change notification settings - Fork 13.4k
BitSet
related perf improvements
#97863
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
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 9fc58edd46d568c75affa6a27b1c4c77dbbe4cdc with merge 18b9172199f08c174c4f5a8540efd1e8ef0bdb12... |
☀️ Try build successful - checks-actions |
Queued 18b9172199f08c174c4f5a8540efd1e8ef0bdb12 with parent 64a7aa7, future comparison URL. |
Finished benchmarking commit (18b9172199f08c174c4f5a8540efd1e8ef0bdb12): comparison url. Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results
CyclesThis benchmark run did not return any relevant results for this metric. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Footnotes |
Wow, that's rather surprising, I expected this code to be perf sensitive. In any case, I'd like to merge this as it brings benefits in destprop which I assume will get merged and turned on at some point |
The code looks reasonable, but we'll need some tests added to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes need, see above.
This commit makes two changes: 1. Changes `MaybeLiveLocals` to use `ChunkedBitSet` 2. Overrides the `fold` method for the iterator for `ChunkedBitSet`
9fc58ed
to
bc7cd2f
Compare
Addressed comments @rustbot ready |
@bors r+ |
📌 Commit bc7cd2f has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (ecdd374): comparison url. Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
This commit makes two changes:
MaybeLiveLocals
to useChunkedBitSet
fold
method for the iterator forChunkedBitSet
I have local benchmarks verifying that each of these changes individually yield significant perf improvements to #96451 . I'm hoping this will be true outside of that context too. If that is not the case, I'll try to gate things on where they help as needed
r? @nnethercote who I believe was working on closely related things, cc @tmiasko because of the destprop pr