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 consider places with the same local in each_borrow_involving_path. #111753

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

cjgillot
Copy link
Contributor

This avoids having a busy loop that repeatedly checks for equality of locals.

@rustbot
Copy link
Collaborator

rustbot commented May 19, 2023

r? @eholk

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 19, 2023
@cjgillot
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

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

bors commented May 19, 2023

⌛ Trying commit 85fe5a0 with merge b2e1ec8f91d4864924d7bc660e4e1714e25c4998...

@rust-log-analyzer

This comment has been minimized.

@cjgillot
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 19, 2023

⌛ Trying commit d84c14603343c01f0d65ef543dc228fa8d04beaa with merge c025715001dea716a1fd153ab20ee67fa5aa9530...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 19, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 19, 2023
@cjgillot
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 19, 2023

⌛ Trying commit b49aba8 with merge 25f421f60574a694b8971f72bba1116a37bd9eae...

@bors
Copy link
Contributor

bors commented May 19, 2023

☀️ Try build successful - checks-actions
Build commit: 25f421f60574a694b8971f72bba1116a37bd9eae (25f421f60574a694b8971f72bba1116a37bd9eae)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (25f421f60574a694b8971f72bba1116a37bd9eae): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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 may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.4% [0.2%, 0.5%] 5
Regressions ❌
(secondary)
0.8% [0.6%, 1.0%] 4
Improvements ✅
(primary)
-0.4% [-0.5%, -0.4%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [-0.5%, 0.5%] 7

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This 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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.5% [4.3%, 4.7%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.4% [-2.4%, -2.4%] 1
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 640.861s -> 641.156s (0.05%)

@rustbot rustbot added the perf-regression Performance regression. label May 19, 2023
@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 19, 2023
@cjgillot cjgillot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 26, 2023
@wesleywiser
Copy link
Member

r? rust-lang/compiler

@rustbot rustbot assigned compiler-errors and unassigned eholk Jul 6, 2023
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 31, 2023

📌 Commit b798939 has been approved by compiler-errors

It is now in the queue for this repository.

@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 Jul 31, 2023
@bors
Copy link
Contributor

bors commented Aug 1, 2023

⌛ Testing commit b798939 with merge 866710c...

@bors
Copy link
Contributor

bors commented Aug 1, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 866710c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 1, 2023
@bors bors merged commit 866710c into rust-lang:master Aug 1, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Aug 1, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (866710c): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.4% [0.2%, 0.6%] 6
Regressions ❌
(secondary)
1.9% [0.7%, 5.9%] 5
Improvements ✅
(primary)
-0.4% [-0.5%, -0.4%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [-0.5%, 0.6%] 8

Max RSS (memory usage)

Results

This 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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.5% [1.5%, 1.5%] 1
Improvements ✅
(primary)
-1.6% [-2.0%, -1.2%] 2
Improvements ✅
(secondary)
-3.5% [-3.5%, -3.5%] 1
All ❌✅ (primary) -1.6% [-2.0%, -1.2%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 649.854s -> 650.5s (0.10%)

@compiler-errors
Copy link
Member

compiler-errors commented Aug 1, 2023

Ugh, didn't catch the perf results when reviewing this. Was too focused on the actual changes to the code 😅

I guess if it's not a perf improvement, we should probably revert this.

@compiler-errors compiler-errors mentioned this pull request Aug 1, 2023
@cjgillot cjgillot deleted the simp-place-conflict branch August 2, 2023 07:15
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. perf-regression Performance regression. 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.

8 participants