Skip to content

[perf only] Drop incremental compilation support #94268

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

Closed
wants to merge 1 commit into from

Conversation

Mark-Simulacrum
Copy link
Member

This removes the majority (all that I could easily find) of incremental support
from the compiler. This is not intended to land on master - but is rather used
for perf to evaluate the impact incremental has on non-incremental builds, which
may provide useful insight into where we can better optimize the conditionals
and such that gate incremental today.

Local results estimate near-zero impact -- around 0.5% -- on non-incremental
builds, obviously making incremental builds slower by a good bit. It'll be interesting
to see the bootstrap time impact, too; I have not measured that locally.

This probably misses anything that's not completely trivially dead code, but from at least the hot path of query code there's definitely nothing incremental-related left.

r? @ghost

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 22, 2022
@Mark-Simulacrum
Copy link
Member Author

@bors try @rust-timer queue

This'll take forever since there's a bunch of incremental tests it'll basically be doing from scratch, but I think we can wait more easily than try to land support for special handling.

@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 Feb 22, 2022
@bors
Copy link
Collaborator

bors commented Feb 22, 2022

⌛ Trying commit d43a71721abab1706e0647706d460e129ab6a381 with merge 1b51d9cec34ac77893b44f41de1a089a44c5b01d...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Feb 22, 2022

💔 Test failed - checks-actions

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 22, 2022
@Mark-Simulacrum
Copy link
Member Author

@bors try

@bors
Copy link
Collaborator

bors commented Feb 23, 2022

⌛ Trying commit 3f473eef05efe8af3b5b47e68128e43b84f87b00 with merge 928c2d96572e23c187b113bf6075cdbc2e59c551...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Feb 23, 2022

💔 Test failed - checks-actions

This removes the majority (all that I could easily find) of incremental support
from the compiler. This is not intended to land on master - but is rather used
for perf to evaluate the impact incremental has on non-incremental builds, which
may provide useful insight into where we can better optimize the conditionals
and such that gate incremental today.
@Mark-Simulacrum
Copy link
Member Author

@bors try

@bors
Copy link
Collaborator

bors commented Feb 23, 2022

⌛ Trying commit 106f827 with merge 08f996275290797d9c0192603890eb7ae11d7ebd...

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking rustc_borrowck v0.0.0 (/checkout/compiler/rustc_borrowck)
    Checking rustc_mir_transform v0.0.0 (/checkout/compiler/rustc_mir_transform)
    Checking rustc_privacy v0.0.0 (/checkout/compiler/rustc_privacy)
    Checking rustc_interface v0.0.0 (/checkout/compiler/rustc_interface)
error[E0107]: this struct takes 1 lifetime argument but 2 lifetime arguments were supplied
     |
     |
163  |     let icx: &tls::ImplicitCtxt<'_, '_> = &*(context as *const tls::ImplicitCtxt<'_, '_>);
     |                    ^^^^^^^^^^^^     -- help: remove this lifetime argument
     |                    expected 1 lifetime argument
     |
     |
note: struct defined here, with 1 lifetime parameter: `'tcx`
    --> /checkout/compiler/rustc_middle/src/ty/context.rs:1706:16
1706 |     pub struct ImplicitCtxt<'tcx> {
     |                ^^^^^^^^^^^^ ----


error[E0107]: this struct takes 1 lifetime argument but 2 lifetime arguments were supplied
     |
     |
162  |     rustc_data_structures::sync::assert_sync::<tls::ImplicitCtxt<'_, '_>>();
     |                                                     ^^^^^^^^^^^^     -- help: remove this lifetime argument
     |                                                     expected 1 lifetime argument
     |
     |
note: struct defined here, with 1 lifetime parameter: `'tcx`
    --> /checkout/compiler/rustc_middle/src/ty/context.rs:1706:16
1706 |     pub struct ImplicitCtxt<'tcx> {
     |                ^^^^^^^^^^^^ ----


error[E0107]: this struct takes 1 lifetime argument but 2 lifetime arguments were supplied
     |
     |
163  |     let icx: &tls::ImplicitCtxt<'_, '_> = &*(context as *const tls::ImplicitCtxt<'_, '_>);
     |                                                                     ^^^^^^^^^^^^     -- help: remove this lifetime argument
     |                                                                     expected 1 lifetime argument
     |
     |
note: struct defined here, with 1 lifetime parameter: `'tcx`
    --> /checkout/compiler/rustc_middle/src/ty/context.rs:1706:16
1706 |     pub struct ImplicitCtxt<'tcx> {
     |                ^^^^^^^^^^^^ ----

For more information about this error, try `rustc --explain E0107`.

@bors
Copy link
Collaborator

bors commented Feb 23, 2022

☀️ Try build successful - checks-actions
Build commit: 08f996275290797d9c0192603890eb7ae11d7ebd (08f996275290797d9c0192603890eb7ae11d7ebd)

@rust-timer
Copy link
Collaborator

Queued 08f996275290797d9c0192603890eb7ae11d7ebd with parent c651ba8, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (08f996275290797d9c0192603890eb7ae11d7ebd): comparison url.

Summary: This benchmark run shows 261 relevant improvements 🎉 but 221 relevant regressions 😿 to instruction counts.

  • Average relevant regression: 458.1%
  • Average relevant improvement: -8.9%
  • Largest improvement in instruction counts: -48.8% on incr-full builds of clap-rs check
  • Largest regression in instruction counts: 4232.4% on incr-unchanged builds of webrender opt

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. 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 Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

5 participants