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

shave 150ms off bootstrap #131954

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Oct 20, 2024

This starts git commands inside GitInfoand the submodule updates in parallel. Git should already perform internal locking in cases where it needs to serialize a modification.

OLD
Benchmark #1: ./x check core
  Time (mean ± σ):     608.7 ms ±   4.4 ms    [User: 368.3 ms, System: 455.1 ms]
  Range (min … max):   602.3 ms … 618.8 ms    10 runs

NEW
Benchmark #1: ./x check core
  Time (mean ± σ):     462.8 ms ±   2.6 ms    [User: 350.2 ms, System: 485.1 ms]
  Range (min … max):   457.5 ms … 465.6 ms    10 runs

This should help with the rust-analyzer setup which issues many individual ./x check calls. There's more that could be done but these were the lowest-hanging fruits that I saw.

@rustbot
Copy link
Collaborator

rustbot commented Oct 20, 2024

r? @Kobzol

rustbot has assigned @Kobzol.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Oct 20, 2024
src/bootstrap/src/utils/helpers.rs Outdated Show resolved Hide resolved
src/bootstrap/src/lib.rs Show resolved Hide resolved
this saves about 150ms on many ./x invocations
@Kobzol
Copy link
Contributor

Kobzol commented Oct 21, 2024

I saw only about ~40 ms improvement locally, but this might be worse for HDDs and/or more complex git index states. Even though this creates a new output function, it does not complicate the code very much and seems like it can have useful perf. benefits. Let's land this for now.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 21, 2024

📌 Commit a269e4d has been approved by Kobzol

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 Oct 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 21, 2024
…bzol

shave 150ms off bootstrap

This starts `git` commands inside `GitInfo`and the submodule updates in parallel. Git should already perform internal locking in cases where it needs to serialize a modification.

```
OLD
Benchmark #1: ./x check core
  Time (mean ± σ):     608.7 ms ±   4.4 ms    [User: 368.3 ms, System: 455.1 ms]
  Range (min … max):   602.3 ms … 618.8 ms    10 runs

NEW
Benchmark #1: ./x check core
  Time (mean ± σ):     462.8 ms ±   2.6 ms    [User: 350.2 ms, System: 485.1 ms]
  Range (min … max):   457.5 ms … 465.6 ms    10 runs
```

This should help with the rust-analyzer setup which issues many individual `./x check` calls. There's more that could be done but these were the lowest-hanging fruits that I saw.
@bors
Copy link
Contributor

bors commented Oct 21, 2024

⌛ Testing commit a269e4d with merge 39f707f...

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Oct 21, 2024

💔 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 Oct 21, 2024
@the8472
Copy link
Member Author

the8472 commented Oct 21, 2024

seems spurious. @bors retry

@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 Oct 21, 2024
fmease added a commit to fmease/rust that referenced this pull request Oct 22, 2024
…Kobzol

shave 150ms off bootstrap

This starts `git` commands inside `GitInfo`and the submodule updates in parallel. Git should already perform internal locking in cases where it needs to serialize a modification.

```
OLD
Benchmark #1: ./x check core
  Time (mean ± σ):     608.7 ms ±   4.4 ms    [User: 368.3 ms, System: 455.1 ms]
  Range (min … max):   602.3 ms … 618.8 ms    10 runs

NEW
Benchmark #1: ./x check core
  Time (mean ± σ):     462.8 ms ±   2.6 ms    [User: 350.2 ms, System: 485.1 ms]
  Range (min … max):   457.5 ms … 465.6 ms    10 runs
```

This should help with the rust-analyzer setup which issues many individual `./x check` calls. There's more that could be done but these were the lowest-hanging fruits that I saw.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#129935 (make unsupported_calling_conventions a hard error)
 - rust-lang#130432 (rust_for_linux: -Zregparm=<N> commandline flag for X86 (rust-lang#116972))
 - rust-lang#131697 (`rt::Argument`: elide lifetimes)
 - rust-lang#131954 (shave 150ms off bootstrap)
 - rust-lang#131982 (Represent `hir::TraitBoundModifiers` as distinct parts in HIR)
 - rust-lang#132017 (Update triagebot.toml)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2024
…kingjubilee

Rollup of 6 pull requests

Successful merges:

 - rust-lang#130432 (rust_for_linux: -Zregparm=<N> commandline flag for X86 (rust-lang#116972))
 - rust-lang#131697 (`rt::Argument`: elide lifetimes)
 - rust-lang#131807 (Always specify `llvm_abiname` for RISC-V targets)
 - rust-lang#131954 (shave 150ms off bootstrap)
 - rust-lang#132015 (Move const trait tests from `ui/rfcs/rfc-2632-const-trait-impl` to `ui/traits/const-traits`)
 - rust-lang#132017 (Update triagebot.toml)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit aba227b into rust-lang:master Oct 22, 2024
6 of 7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2024
Rollup merge of rust-lang#131954 - the8472:bootstrap-parallel-git, r=Kobzol

shave 150ms off bootstrap

This starts `git` commands inside `GitInfo`and the submodule updates in parallel. Git should already perform internal locking in cases where it needs to serialize a modification.

```
OLD
Benchmark #1: ./x check core
  Time (mean ± σ):     608.7 ms ±   4.4 ms    [User: 368.3 ms, System: 455.1 ms]
  Range (min … max):   602.3 ms … 618.8 ms    10 runs

NEW
Benchmark #1: ./x check core
  Time (mean ± σ):     462.8 ms ±   2.6 ms    [User: 350.2 ms, System: 485.1 ms]
  Range (min … max):   457.5 ms … 465.6 ms    10 runs
```

This should help with the rust-analyzer setup which issues many individual `./x check` calls. There's more that could be done but these were the lowest-hanging fruits that I saw.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants