Skip to content
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

Only traverse attrs once while checking for coherence override attributes #93343

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

lqd
Copy link
Member

@lqd lqd commented Jan 26, 2022

In coherence, while checking for negative impls override attributes: only traverse the DefIds' attributes once.

This PR is an easy way to get back some of the small perf loss in #93175

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 26, 2022
@lqd
Copy link
Member Author

lqd commented Jan 26, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 26, 2022
@bors
Copy link
Contributor

bors commented Jan 26, 2022

⌛ Trying commit 644c22d with merge 14b4e3475978414bd2231200da745dc72f659842...

@bors
Copy link
Contributor

bors commented Jan 26, 2022

☀️ Try build successful - checks-actions
Build commit: 14b4e3475978414bd2231200da745dc72f659842 (14b4e3475978414bd2231200da745dc72f659842)

@rust-timer
Copy link
Collaborator

Queued 14b4e3475978414bd2231200da745dc72f659842 with parent a7f3757, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (14b4e3475978414bd2231200da745dc72f659842): comparison url.

Summary: This benchmark run shows 7 relevant improvements 🎉 to instruction counts.

  • Average relevant improvement: -1.1%
  • Largest improvement in instruction counts: -1.4% on incr-full builds of stm32f4 check

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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 26, 2022
@lqd lqd changed the title [PERF EXPERIMENT] Only traverse attrs once while checking for coherence override attributes Only traverse attrs once while checking for coherence override attributes Jan 26, 2022
@lqd
Copy link
Member Author

lqd commented Jan 26, 2022

Super quick way to get back some of the small losses in the negative impl PR #93175, so r? @spastorino to do what you want with it, especially when #93348 should be better than this

@spastorino
Copy link
Member

@bors r+

Let's land this but I'm taking a different approach in another PR. Would be nice to see performance difference with that other approach too.

@bors
Copy link
Contributor

bors commented Jan 27, 2022

📌 Commit 644c22d has been approved by spastorino

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 27, 2022
@bors
Copy link
Contributor

bors commented Jan 28, 2022

⌛ Testing commit 644c22d with merge 312a799...

@bors
Copy link
Contributor

bors commented Jan 28, 2022

☀️ Test successful - checks-actions
Approved by: spastorino
Pushing 312a799 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 28, 2022
@bors bors merged commit 312a799 into rust-lang:master Jan 28, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 28, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (312a799): comparison url.

Summary: This benchmark run shows 7 relevant improvements 🎉 to instruction counts.

  • Average relevant improvement: -1.1%
  • Largest improvement in instruction counts: -1.4% on incr-full builds of stm32f4 check

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@lqd lqd deleted the attrs branch January 28, 2022 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants