-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Stop emitting impls for PartialEq::ne and PartialOrd::{lt, le, gt, ge} #64065
Stop emitting impls for PartialEq::ne and PartialOrd::{lt, le, gt, ge} #64065
Conversation
⌛ Trying commit d3ae62aef6f483809d380ac31a92aa9705d1cb00 with merge b2d4196c0b1d453ff12c354918674dea88f8272b... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
d3ae62a
to
9f2162c
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
These are never better optimized than calling the main method (eq, partial_cmp) in the derived case. This makes us generate far less code, which is a compile time win.
9f2162c
to
23f30da
Compare
@bors try |
Stop emitting impls for PartialEq::ne and PartialOrd::{lt, le, gt, ge} These are never better optimized than calling the main method (eq, partial_cmp) in the derived case. This makes us generate far less code, which is a compile time win. r? @ghost Mostly opening this so that I can run perf on it to get a loose sense of whether this is worth it or not.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☀️ Try build successful - checks-azure |
@rust-timer build 44bcb9c |
Success: Queued 44bcb9c with parent d0677b9, comparison URL. |
Finished benchmarking try commit 44bcb9c, comparison URL. |
This appears to be mostly a wash performance wise at least on the tests we have in-tree so I'm going to close this and the associated issue. |
These are never better optimized than calling the main method (eq,
partial_cmp) in the derived case. This makes us generate far less code,
which is a compile time win.
r? @ghost
Mostly opening this so that I can run perf on it to get a loose sense of whether this is worth it or not.