-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Modify the buffer position directly when reading leb128 values. #92631
Conversation
It's a small but clear performance win.
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 5f549d9 with merge 50c7ddde3ce168b8c3ba9a790e49d8df73d08fdd... |
☀️ Try build successful - checks-actions |
Queued 50c7ddde3ce168b8c3ba9a790e49d8df73d08fdd with parent cfa4ac6, future comparison URL. |
Finished benchmarking commit (50c7ddde3ce168b8c3ba9a790e49d8df73d08fdd): comparison url. Summary: This change led to small relevant improvements 🎉 in compiler performance.
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 led to changes in compiler perf. @bors rollup=never |
The CI results show universal instruction count improvements, but they are smaller than what I saw locally, and spread differently across the benchmarks. Here are the top 10
And here they from my local measurements (where I only did
I feel like the match-ups between my local measurements and CI measurements are much weaker than they used to be. I wonder if that's because I don't do PGO locally. |
Thanks, @nnethercote! Looks good to me. @bors r+
Yeah, I suspect that that plays into it. |
📌 Commit 5f549d9 has been approved by |
Closing in favor of #92604 |
It's a small but clear performance win.
r? @michaelwoerister