-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Optimize proc macro bridge #85390
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
Optimize proc macro bridge #85390
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 4df5d58e6e7ca658565a0ffc9853cd791788ea29 with merge 55c4f6ccc851db7f04cd03a160665208bcaa65ea... |
☀️ Try build successful - checks-actions |
Queued 55c4f6ccc851db7f04cd03a160665208bcaa65ea with parent fe72845, future comparison URL. |
Finished benchmarking try commit (55c4f6ccc851db7f04cd03a160665208bcaa65ea): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
I don’t think adjustment on ra side is required, but we’ll want to port this over some day to keep the code identical. I wonder if we should look into sharing the impl here via a published crate, and not via copy-paste. cc @edwin0cheng |
copy_from_slice generally falls back to memcpy/memmove, which is much more expensive than we need to write a single element in. This saves 0.26% instructions on the diesel benchmark.
This is a 0.15% win on diesel.
This allows a more efficient implementation (avoiding a fallback to memmove, which is not optimal for short writes). This saves 0.29% on diesel.
4df5d58
to
8c20808
Compare
@bors r+ |
📌 Commit 8c20808 has been approved by |
⌛ Testing commit 8c20808 with merge 73f497f7caef0a3168037b452f46917b93c85590... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry thread 'main' panicked at 'client.read_exact(&mut header) failed with Connection reset by peer (os error 104)', src/tools/remote-test-client/src/main.rs:310:9 |
☀️ Test successful - checks-actions |
extend_from_slice: extern "C" fn(Buffer<T>, Slice<'_, T>) -> Buffer<T>, | ||
reserve: extern "C" fn(Buffer<T>, usize) -> Buffer<T>, |
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.
This is the last use of Slice
and now Slice
can be removed (not sure why it didn't warn/error as unused).
proc_macro/bridge: Remove dead code Slice type See rust-lang#85390 (comment)
proc_macro/bridge: Remove dead code Slice type See rust-lang#85390 (comment)
This optimizes the proc macro bridge code for a win of 0.7% instruction counts on the diesel-check benchmark (non-incr, full). These wins are small, but hopefully not limited to just the diesel benchmark; the code is also not seriously impacted by the changes here.