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

Revert "Compute lint levels by definition" #101839

Closed
wants to merge 1 commit into from

Conversation

cjgillot
Copy link
Contributor

Reverts #101620

The final perf results are very different from the measurement during review.

r? @oli-obk

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 15, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 15, 2022
@cjgillot
Copy link
Contributor Author

Measurement during review: https://perf.rust-lang.org/compare.html?start=98f3001eecbe4cbd091c10ffab45b4c164bb507b&end=4ae314edea36bbec3e6a73de890c1d6c727690ac&stat=instructions:u

  mean1 range count2
Regressions (primary) 1.0% [0.2%, 2.6%] 24
Regressions (secondary) 1.6% [0.2%, 4.0%] 35
Improvements (primary) -4.4% [-28.2%, -0.2%] 57
Improvements (secondary) -0.9% [-2.0%, -0.2%] 42
All (primary) -2.8% [-28.2%, 2.6%] 81

Post-merge: https://perf.rust-lang.org/compare.html?start=750bd1a7ff3e010611b97ee75d30b7cbf5f3a03c&end=2cb9a65684dba47c52de8fa938febf97a73e70a9&stat=instructions:u

  mean1 range count2
Regressions (primary) 4.1% [0.4%, 14.5%] 133
Regressions (secondary) 5.2% [0.2%, 64.1%] 131
Improvements (primary) -5.1% [-24.3%, -0.4%] 30
Improvements (secondary) -1.0% [-1.2%, -0.5%] 5
All (primary) 2.4% [-24.3%, 14.5%] 163

@cjgillot
Copy link
Contributor Author

@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 Sep 15, 2022
@bors
Copy link
Collaborator

bors commented Sep 15, 2022

⌛ Trying commit 18066f1 with merge 210412ff6028dddbd084275dd1cd304864ed7ca2...

@bors
Copy link
Collaborator

bors commented Sep 15, 2022

☀️ Try build successful - checks-actions
Build commit: 210412ff6028dddbd084275dd1cd304864ed7ca2 (210412ff6028dddbd084275dd1cd304864ed7ca2)

@rust-timer
Copy link
Collaborator

Queued 210412ff6028dddbd084275dd1cd304864ed7ca2 with parent c3f5929, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (210412ff6028dddbd084275dd1cd304864ed7ca2): 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-review -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.

mean1 range count2
Regressions ❌
(primary)
5.7% [0.2%, 32.2%] 31
Regressions ❌
(secondary)
0.9% [0.2%, 1.3%] 7
Improvements ✅
(primary)
-3.8% [-12.5%, -0.4%] 132
Improvements ✅
(secondary)
-4.4% [-39.3%, -0.2%] 135
All ❌✅ (primary) -2.0% [-12.5%, 32.2%] 163

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.

mean1 range count2
Regressions ❌
(primary)
4.3% [1.1%, 9.9%] 12
Regressions ❌
(secondary)
4.0% [3.0%, 5.0%] 2
Improvements ✅
(primary)
-4.2% [-13.0%, -1.0%] 101
Improvements ✅
(secondary)
-8.1% [-22.3%, -1.4%] 65
All ❌✅ (primary) -3.3% [-13.0%, 9.9%] 113

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.

mean1 range count2
Regressions ❌
(primary)
15.1% [2.4%, 45.1%] 17
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-5.0% [-15.9%, -1.3%] 86
Improvements ✅
(secondary)
-9.1% [-31.9%, -1.9%] 71
All ❌✅ (primary) -1.7% [-15.9%, 45.1%] 103

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Sep 15, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Sep 15, 2022

@bors r+ rollup=never

@bors
Copy link
Collaborator

bors commented Sep 15, 2022

📌 Commit 18066f1 has been approved by oli-obk

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 Sep 15, 2022
@bors
Copy link
Collaborator

bors commented Sep 15, 2022

⌛ Testing commit 18066f1 with merge a7dd7dcbba0333878ec10067bb3253b1ef7ca0de...

@bors
Copy link
Collaborator

bors commented Sep 16, 2022

💔 Test failed - checks-actions

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

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] fluent_bundle test:false 1.925
   Compiling rustc_macros v0.1.0 (/checkout/compiler/rustc_macros)
   Compiling tracing-tree v0.2.0
[RUSTC-TIMING] chalk_ir test:false 5.483
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.

@cjgillot
Copy link
Contributor Author

Replaced by #101862 with an actual fix.

@cjgillot cjgillot closed this Sep 16, 2022
@cjgillot cjgillot reopened this Sep 16, 2022
@bors
Copy link
Collaborator

bors commented Sep 17, 2022

☔ The latest upstream changes (presumably #101938) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk oli-obk closed this Sep 19, 2022
@cjgillot cjgillot deleted the revert-101620-compute_lint_levels_by_def branch September 19, 2022 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) perf-regression Performance regression. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants