-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
[rustdoc] Remove unneeded clone()
calls for derive_id
#114204
[rustdoc] Remove unneeded clone()
calls for derive_id
#114204
Conversation
I'm not sure it'll impact performance but better confirm: @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
⌛ Trying commit f65dc084ceb7f562668ea249391b138d1303dfe6 with merge 5234c646c74d83bfccd3f219d68d867dd0b877f8... |
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (5234c646c74d83bfccd3f219d68d867dd0b877f8): comparison URL. Overall result: no relevant changes - no action neededBenchmarking 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 Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 653.409s -> 652.628s (-0.12%) |
f65dc08
to
148a0c1
Compare
It's based both on the historical data and the magnitude of change. So here it was determined that these benchmarks were "noisy enough" recently and that the current change was not large enough in magnitude for it to be deemed relevant. |
I see, makes sense. Thanks for the clarification. |
It’s fine. Removing unnecessary clones is just good practice anyway. @bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (a8be6e0): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 651.166s -> 651.129s (-0.01%) |
I realized we were cloning values before passing them to
derive_id
where they are cloned again, which isn't great. Since they'll be cloned anyway, let's allow to pass both by reference and by value.r? @notriddle