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.

Sign up for GitHub

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

Jump to bottom

Prevent constant rebuilds of rustc-main (and thus everything else) #114613

Merged
merged 1 commit into from
Aug 9, 2023
Merged

Prevent constant rebuilds of rustc-main (and thus everything else) #114613

merged 1 commit into from
Aug 9, 2023

Conversation

pietroalbini
Copy link
Member

PR #114305 changed bootstrap to run strip -g on librustc_driver.so and libllvm.so on Linux when no debuginfo was requested. Unfortunately, that PR resulted in bootstrap always rebuilding everything starting from stage 1 rustc-main (including stage 1 libraries and tests) when invoking bootstrap multiple times.

We noticed this because Ferrocene's CI times increased to between 2x and 3x total execution time, but the regression can also be reproduced locally by running ./x build library/sysroot --stage 1 twice.

The explanation of the problem is in the code comments.

r? @lqd
cc @ozkanonur

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Aug 8, 2023
@rustbot
Copy link
Collaborator

rustbot commented Aug 8, 2023

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

lqd
lqd reviewed Aug 8, 2023
View reviewed changes
Copy link
Member

@lqd lqd left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this was missed in the initial PR.

I've tested this and stripping still works locally on both llvm and rustc_driver.

r=me with the nit fixed

src/bootstrap/llvm.rs Outdated Show resolved Hide resolved
src/bootstrap/compile.rs Show resolved Hide resolved
@Kobzol
Copy link
Contributor

Kobzol commented Aug 8, 2023

We had the exact same issue with BOLT. I solved it with doing the BOLT modification on-the-fly in the dist step, and later just moved the BOLT logic to opt-dist, to do this outside of bootstrap. Since I think that the debug strip logic is basically a duct tape patch for a specific situation on 64-bit Linux, I think that we should also eventually move this to opt-dist.

@pietroalbini
Copy link
Member Author

@bors r=lqd rollup

@bors
Copy link
Contributor

bors commented Aug 8, 2023

📌 Commit da00356 has been approved by lqd

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 Aug 8, 2023
@Dylan-DPC Dylan-DPC mentioned this pull request Aug 8, 2023
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 8, 2023
@Dylan-DPC
Rollup merge of rust-lang#114613 - ferrocene:pa-fix-rebuild, r=lqd
a3fb831 
Prevent constant rebuilds of `rustc-main` (and thus everything else)

PR rust-lang#114305 changed bootstrap to run `strip -g` on `librustc_driver.so` and `libllvm.so` on Linux when no debuginfo was requested. Unfortunately, that PR resulted in bootstrap always rebuilding everything starting from stage 1 `rustc-main` (including stage 1 libraries and tests) when invoking bootstrap multiple times.

We noticed this because Ferrocene's CI times increased to between 2x and 3x total execution time, but the regression can also be reproduced locally by running `./x build library/sysroot --stage 1` twice.

The explanation of the problem is in the code comments.

r? `@lqd`
cc `@ozkanonur`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 8, 2023
@matthiaskrgr
Rollup merge of rust-lang#114613 - ferrocene:pa-fix-rebuild, r=lqd
16f27fd 
Prevent constant rebuilds of `rustc-main` (and thus everything else)

PR rust-lang#114305 changed bootstrap to run `strip -g` on `librustc_driver.so` and `libllvm.so` on Linux when no debuginfo was requested. Unfortunately, that PR resulted in bootstrap always rebuilding everything starting from stage 1 `rustc-main` (including stage 1 libraries and tests) when invoking bootstrap multiple times.

We noticed this because Ferrocene's CI times increased to between 2x and 3x total execution time, but the regression can also be reproduced locally by running `./x build library/sysroot --stage 1` twice.

The explanation of the problem is in the code comments.

r? ``@lqd``
cc ``@ozkanonur``
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 8, 2023
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 8, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 8, 2023
@bors
Auto merge of rust-lang#114637 - matthiaskrgr:rollup-544y8p5, r=matth…
e3590fc 
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#106425 (Make ExitStatus implement Default)
 - rust-lang#113480 (add aarch64-unknown-teeos target)
 - rust-lang#113586 (Mention style for new syntax in tracking issue template)
 - rust-lang#113593 (CFI: Fix error compiling core with LLVM CFI enabled)
 - rust-lang#114612 (update llvm-wrapper include to silence deprecation warning)
 - rust-lang#114613 (Prevent constant rebuilds of `rustc-main` (and thus everything else))
 - rust-lang#114615 (interpret: remove incomplete protection against invalid where clauses)
 - rust-lang#114628 (Allowing re-implementation of mir_drops_elaborated query)
 - rust-lang#114629 (tests: Uncomment now valid GAT code behind FIXME)
 - rust-lang#114630 (Migrate GUI colors test to original CSS color format)
 - rust-lang#114631 (add provisional cache test for new solver)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4f82fb8 into rust-lang:master Aug 9, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Aug 9, 2023
@pietroalbini pietroalbini deleted the pa-fix-rebuild branch August 9, 2023 07:17
@klensy klensy mentioned this pull request Aug 9, 2023
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 13, 2023
@GuillaumeGomez
Rollup merge of rust-lang#114625 - klensy:ci-pr-workers, r=Mark-Simul…
d868276 
…acrum

CI: use smaller machines in PR runs

mingw-check job-linux-16c -> job-linux-4c
~job-linux-4c 20 min in auto job
~job-linux-16c 13 min in pr job
with current pr regressed to almost 21 min, it's ok.

mingw-check-tidy job-linux-16c -> job-linux-4c small enough, so reduce to minimal
~ job-linux-16c 3 min
with current pr regressed to almost 5 min, it's ok.

x86_64-gnu-tools job-linux-16c this is top job by time in PR, so don't touch it
~ job-linux-8c 1.30 hour in auto job
~ job-linux-16c 1 hour in pr job (affected by rust-lang#114613, actual time ~ 30 min)

x86_64-gnu-llvm-15 job-linux-16c don't change too
~ job-linux-8c 1.30 hour in auto job
~ job-linux-16c 30 min in pr job

Noticed while working on rust-lang#114621, so current time affected by always rebuilded docker images (but pr images always rebuilded before too, so nvm)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lqd lqd lqd left review comments

Assignees

@lqd lqd

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.73.0
Development

Successfully merging this pull request may close these issues.
5 participants
@pietroalbini @rustbot @Kobzol @bors @lqd