Uh oh!
There was an error while loading. Please reload this page.
perf: only check for rustc_trivial_field_reads attribute on traits, not items, impls, etc. - #89454
Conversation
The checks removed here caused a small perf regression: rust-lang#88824 (comment) Since the attribute is currently only applied to traits, I don't think it's worth keeping the additional checks for now. If/when we decide to apply the attribute somewhere else, we can (partially) revert this and evaluate if the perf impact is acceptable.
Mark-Simulacrum
commented
Oct 2, 2021
@bors try @rust-timer queue |
rust-timer
commented
Oct 2, 2021
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
bors
commented
Oct 2, 2021
⌛ Trying commit eb4ba58 with merge 6af27bf535758296bda5cf8bf2a01bd4b1764c1a... |
bors
commented
Oct 2, 2021
☀️ Try build successful - checks-actions |
rust-timer
commented
Oct 2, 2021
Queued 6af27bf535758296bda5cf8bf2a01bd4b1764c1a with parent 6e12110, future comparison URL. |
rust-timer
commented
Oct 2, 2021
Finished benchmarking commit (6af27bf535758296bda5cf8bf2a01bd4b1764c1a): comparison url. Summary: This change led to moderate 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 |
erikdesjardins
commented
Oct 2, 2021
Can I get another perf run? |
Mark-Simulacrum
commented
Oct 2, 2021
@bors try @rust-timer queue |
rust-timer
commented
Oct 2, 2021
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
bors
commented
Oct 2, 2021
⌛ Trying commit bec5a91 with merge c5f130816ee1458aa21bba39bf47f9c849e696a6... |
bors
commented
Oct 2, 2021
☀️ Try build successful - checks-actions |
rust-timer
commented
Oct 2, 2021
Queued c5f130816ee1458aa21bba39bf47f9c849e696a6 with parent b27661e, future comparison URL. |
rust-timer
commented
Oct 2, 2021
Finished benchmarking commit (c5f130816ee1458aa21bba39bf47f9c849e696a6): comparison url. Summary: This change led to moderate 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 |
nikomatsakis
commented
Oct 4, 2021
@bors r+ |
bors
commented
Oct 4, 2021
📌 Commit bec5a91 has been approved by |
bors
commented
Oct 6, 2021
⌛ Testing commit bec5a91 with merge f0cbd0e4677561339dd252ac2c1281b69d42ddad... |
bors
commented
Oct 6, 2021
💔 Test failed - checks-actions |
rust-log-analyzer
commented
Oct 6, 2021
A job failed! Check out the build log: (web)(plain) Click to see the possible cause of the failure (guessed by this bot) |
Mark-Simulacrum
commented
Oct 6, 2021
bors
commented
Oct 7, 2021
bors
commented
Oct 7, 2021
☀️ Test successful - checks-actions |
rust-timer
commented
Oct 7, 2021
Finished benchmarking commit (d3e6770): comparison url. Summary: This change led to moderate relevant improvements 🎉 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
The checks that are removed in this PR (originally added in #85200) caused a small perf regression: #88824 (comment)
Since the attribute is currently only applied to traits, I don't think it's worth keeping the additional checks for now.
If/when we decide to apply the attribute somewhere else, we can (partially) revert this and reevaluate the perf impact.
r? @nikomatsakis cc @FabianWolff