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

Compute stability by definition #93017

Closed
wants to merge 3 commits into from

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Jan 17, 2022

The current query is monolithic and depends on all the items in the crate at once. As a consequence, it is always recomputed and walk the whole HIR.

This PR aims to compute this information on-demand only.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 17, 2022
@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 17, 2022
@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 Jan 17, 2022
@bors
Copy link
Contributor

bors commented Jan 17, 2022

⌛ Trying commit 59a56a84b9bc8d24ed1c79f2b06316aa878ad9d7 with merge 3c5fb9f44bb793a329827fbbe81bce24ba6e81b6...

@bors
Copy link
Contributor

bors commented Jan 17, 2022

☀️ Try build successful - checks-actions
Build commit: 3c5fb9f44bb793a329827fbbe81bce24ba6e81b6 (3c5fb9f44bb793a329827fbbe81bce24ba6e81b6)

@rust-timer
Copy link
Collaborator

Queued 3c5fb9f44bb793a329827fbbe81bce24ba6e81b6 with parent ee5d8d3, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3c5fb9f44bb793a329827fbbe81bce24ba6e81b6): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

  • Moderate improvement in instruction counts (up to -1.0% on incr-unchanged builds of ctfe-stress-4 check)
  • Large regression in instruction counts (up to 8.0% on incr-full builds of unused-warnings 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.

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

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 18, 2022
Comment on lines 60 to 61
/// This is mostly a cache, except the stabilities of local items
/// are filled by the annotator.

This comment was marked as resolved.

@rust-log-analyzer

This comment has been minimized.

@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 Jan 19, 2022
@bors
Copy link
Contributor

bors commented Jan 19, 2022

⌛ Trying commit 2881231bc581e82a05d2ab4b8aa863c2820037df with merge 1f4e3b20d63a9454962927681c913350a3599109...

@bors
Copy link
Contributor

bors commented Jan 19, 2022

☀️ Try build successful - checks-actions
Build commit: 1f4e3b20d63a9454962927681c913350a3599109 (1f4e3b20d63a9454962927681c913350a3599109)

@rust-timer
Copy link
Collaborator

Queued 1f4e3b20d63a9454962927681c913350a3599109 with parent 5e57faa, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1f4e3b20d63a9454962927681c913350a3599109): comparison url.

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

  • Moderate improvement in instruction counts (up to -1.1% on incr-unchanged builds of ctfe-stress-4 check)
  • Very large regression in instruction counts (up to 6.0% on incr-unchanged builds of match-stress-enum 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.

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

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

petrochenkov commented Jan 22, 2022

These stability/deprecation checking passes are something that I also wanted to move to name resolution, to fix stability/deprecation checking for reexports and individual components inside paths (cc #15702), if that is done then it will probably still look like a monolithic pass across the whole crate.

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

bors commented Jan 31, 2023

⌛ Trying commit 20ef0110ede1e9262a129e9595f4687a2e28ca6b with merge 34abd002a81d8d98e4aec1d01e31b6c4b94ad09c...

@bors
Copy link
Contributor

bors commented Feb 1, 2023

☀️ Try build successful - checks-actions
Build commit: 34abd002a81d8d98e4aec1d01e31b6c4b94ad09c (34abd002a81d8d98e4aec1d01e31b6c4b94ad09c)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (34abd002a81d8d98e4aec1d01e31b6c4b94ad09c): comparison URL.

Overall result: ❌ regressions - 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.8%] 39
Regressions ❌
(secondary)
2.9% [0.3%, 5.7%] 16
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 2
All ❌✅ (primary) 0.4% [0.2%, 0.8%] 39

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.8% [0.5%, 1.4%] 5
Regressions ❌
(secondary)
3.8% [3.7%, 3.9%] 2
Improvements ✅
(primary)
-3.0% [-3.4%, -2.5%] 3
Improvements ✅
(secondary)
-2.2% [-4.1%, -0.9%] 9
All ❌✅ (primary) -0.6% [-3.4%, 1.4%] 8

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)
5.9% [3.7%, 7.7%] 9
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 1, 2023
@cjgillot
Copy link
Contributor Author

cjgillot commented Feb 1, 2023

With only lookup_deprecation_entry
@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 Feb 1, 2023
@cjgillot
Copy link
Contributor Author

cjgillot commented Feb 2, 2023

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@cjgillot cjgillot closed this Feb 2, 2023
@cjgillot cjgillot reopened this Feb 2, 2023
@cjgillot
Copy link
Contributor Author

cjgillot commented Feb 2, 2023

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 2, 2023

⌛ Trying commit 87f62c3 with merge 3935c8634970bca12aada5357279acd519f03012...

@bors
Copy link
Contributor

bors commented Feb 3, 2023

☀️ Try build successful - checks-actions
Build commit: 3935c8634970bca12aada5357279acd519f03012 (3935c8634970bca12aada5357279acd519f03012)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3935c8634970bca12aada5357279acd519f03012): comparison URL.

Overall result: ❌ regressions - 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.5% [0.2%, 0.9%] 59
Regressions ❌
(secondary)
1.1% [0.1%, 3.0%] 29
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.5% [0.2%, 0.9%] 59

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)
1.5% [0.6%, 4.7%] 8
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.5% [-1.5%, -1.5%] 1
All ❌✅ (primary) 1.5% [0.6%, 4.7%] 8

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)
1.2% [1.2%, 1.2%] 1
Regressions ❌
(secondary)
2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.2% [1.2%, 1.2%] 1

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 3, 2023
@cjgillot cjgillot closed this Feb 3, 2023
@cjgillot cjgillot deleted the stability-query branch February 3, 2023 15:49
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-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.