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

Speed up bootstrap a little. #73352

Merged
merged 4 commits into from
Jun 19, 2020
Merged

Speed up bootstrap a little. #73352

merged 4 commits into from
Jun 19, 2020

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jun 14, 2020

The bootstrap script was calling cargo metadata 3 times (or 6 with -v). This is a very expensive operation, and this attempts to avoid the extra calls. On my system, a simple command like ./x.py test -h -v goes from about 3 seconds to 0.4.

An overview of the changes:

  • Call cargo metadata only once with --no-deps. Optional dependencies are filtered in in_tree_crates (handling profiler_builtins and rustc_codegen_llvm which are driven by the config).
  • Remove a duplicate call to metadata::build when using -v. I'm not sure why it was there, it looks like a mistake or vestigial from previous behavior.
  • Remove check for _shim, I believe all the _shim crates are now gone.
  • Remove check for rustc_ and *san for test::Crate::should_run, these are no longer dependencies in the test tree.
  • Use relative paths in ./x.py test -h -v output.
  • Some code cleanup (remove unnecessary find_compiler_crates, etc.).
  • Show suite paths (src/test/ui/...) in ./x.py test -h -v output.
  • Some doc comments.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(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 Jun 14, 2020
@Mark-Simulacrum
Copy link
Member

I don't know that this performance matters that much - in practice, compiling is going to take a minute or two I think, at least. But I think it's a good cleanup regardless. I think it's fine to hard code profiler builtins and LLVM in bootstrap for the extra perf win as well.

@petrochenkov
Copy link
Contributor

I does matter - running x.py test on a single test is a pretty common operation that performs no-op build (which runs all these operations, as far as I understand), and that no-op build takes some noticeable time, and it would be good to make it faster.

@Mark-Simulacrum
Copy link
Member

Yes, though I think those no-op builds will end up doing something similar to cargo metadata? So it's not as significant a win. Still, seconds shaved off of every build is good.

This should run much faster.

There are also some drive-by cleanups here to try to simplify things.
Also, the paths for in-tree crates are now displayed as relative
in `x.py test -h -v`.
@ehuss
Copy link
Contributor Author

ehuss commented Jun 15, 2020

OK, I have pushed an update that uses --no-deps. I also did a few small drive-by fixes, which I can split off if necessary. They are:

  • Use relative paths in ./x.py test -h -v output.
  • Some code cleanup (remove unnecessary find_compiler_crates, etc.).
  • Show suite paths (src/test/ui/...) in ./x.py test -h -v output.
  • Some doc comments.

@Mark-Simulacrum
Copy link
Member

r=me with the description updated to include the latest changes (profiler builtins and llvm in particular).

@ehuss
Copy link
Contributor Author

ehuss commented Jun 16, 2020

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jun 16, 2020

📌 Commit f17fd7b has been approved by Mark-Simulacrum

@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 Jun 16, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…mulacrum

Speed up bootstrap a little.

The bootstrap script was calling `cargo metadata` 3 times (or 6 with `-v`). This is a very expensive operation, and this attempts to avoid the extra calls. On my system, a simple command like `./x.py test -h -v` goes from about 3 seconds to 0.4.

An overview of the changes:

- Call `cargo metadata` only once with `--no-deps`. Optional dependencies are filtered in `in_tree_crates` (handling `profiler_builtins` and `rustc_codegen_llvm` which are driven by the config).
- Remove a duplicate call to `metadata::build` when using `-v`. I'm not sure why it was there, it looks like a mistake or vestigial from previous behavior.
- Remove check for `_shim`, I believe all the `_shim` crates are now gone.
- Remove check for `rustc_` and `*san` for `test::Crate::should_run`, these are no longer dependencies in the `test` tree.
- Use relative paths in `./x.py test -h -v` output.
- Some code cleanup (remove unnecessary `find_compiler_crates`, etc.).
- Show suite paths (`src/test/ui/...`) in `./x.py test -h -v` output.
- Some doc comments.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…mulacrum

Speed up bootstrap a little.

The bootstrap script was calling `cargo metadata` 3 times (or 6 with `-v`). This is a very expensive operation, and this attempts to avoid the extra calls. On my system, a simple command like `./x.py test -h -v` goes from about 3 seconds to 0.4.

An overview of the changes:

- Call `cargo metadata` only once with `--no-deps`. Optional dependencies are filtered in `in_tree_crates` (handling `profiler_builtins` and `rustc_codegen_llvm` which are driven by the config).
- Remove a duplicate call to `metadata::build` when using `-v`. I'm not sure why it was there, it looks like a mistake or vestigial from previous behavior.
- Remove check for `_shim`, I believe all the `_shim` crates are now gone.
- Remove check for `rustc_` and `*san` for `test::Crate::should_run`, these are no longer dependencies in the `test` tree.
- Use relative paths in `./x.py test -h -v` output.
- Some code cleanup (remove unnecessary `find_compiler_crates`, etc.).
- Show suite paths (`src/test/ui/...`) in `./x.py test -h -v` output.
- Some doc comments.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…mulacrum

Speed up bootstrap a little.

The bootstrap script was calling `cargo metadata` 3 times (or 6 with `-v`). This is a very expensive operation, and this attempts to avoid the extra calls. On my system, a simple command like `./x.py test -h -v` goes from about 3 seconds to 0.4.

An overview of the changes:

- Call `cargo metadata` only once with `--no-deps`. Optional dependencies are filtered in `in_tree_crates` (handling `profiler_builtins` and `rustc_codegen_llvm` which are driven by the config).
- Remove a duplicate call to `metadata::build` when using `-v`. I'm not sure why it was there, it looks like a mistake or vestigial from previous behavior.
- Remove check for `_shim`, I believe all the `_shim` crates are now gone.
- Remove check for `rustc_` and `*san` for `test::Crate::should_run`, these are no longer dependencies in the `test` tree.
- Use relative paths in `./x.py test -h -v` output.
- Some code cleanup (remove unnecessary `find_compiler_crates`, etc.).
- Show suite paths (`src/test/ui/...`) in `./x.py test -h -v` output.
- Some doc comments.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…mulacrum

Speed up bootstrap a little.

The bootstrap script was calling `cargo metadata` 3 times (or 6 with `-v`). This is a very expensive operation, and this attempts to avoid the extra calls. On my system, a simple command like `./x.py test -h -v` goes from about 3 seconds to 0.4.

An overview of the changes:

- Call `cargo metadata` only once with `--no-deps`. Optional dependencies are filtered in `in_tree_crates` (handling `profiler_builtins` and `rustc_codegen_llvm` which are driven by the config).
- Remove a duplicate call to `metadata::build` when using `-v`. I'm not sure why it was there, it looks like a mistake or vestigial from previous behavior.
- Remove check for `_shim`, I believe all the `_shim` crates are now gone.
- Remove check for `rustc_` and `*san` for `test::Crate::should_run`, these are no longer dependencies in the `test` tree.
- Use relative paths in `./x.py test -h -v` output.
- Some code cleanup (remove unnecessary `find_compiler_crates`, etc.).
- Show suite paths (`src/test/ui/...`) in `./x.py test -h -v` output.
- Some doc comments.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…mulacrum

Speed up bootstrap a little.

The bootstrap script was calling `cargo metadata` 3 times (or 6 with `-v`). This is a very expensive operation, and this attempts to avoid the extra calls. On my system, a simple command like `./x.py test -h -v` goes from about 3 seconds to 0.4.

An overview of the changes:

- Call `cargo metadata` only once with `--no-deps`. Optional dependencies are filtered in `in_tree_crates` (handling `profiler_builtins` and `rustc_codegen_llvm` which are driven by the config).
- Remove a duplicate call to `metadata::build` when using `-v`. I'm not sure why it was there, it looks like a mistake or vestigial from previous behavior.
- Remove check for `_shim`, I believe all the `_shim` crates are now gone.
- Remove check for `rustc_` and `*san` for `test::Crate::should_run`, these are no longer dependencies in the `test` tree.
- Use relative paths in `./x.py test -h -v` output.
- Some code cleanup (remove unnecessary `find_compiler_crates`, etc.).
- Show suite paths (`src/test/ui/...`) in `./x.py test -h -v` output.
- Some doc comments.
@RalfJung
Copy link
Member

@bors rollup

Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 19, 2020
…mulacrum

Speed up bootstrap a little.

The bootstrap script was calling `cargo metadata` 3 times (or 6 with `-v`). This is a very expensive operation, and this attempts to avoid the extra calls. On my system, a simple command like `./x.py test -h -v` goes from about 3 seconds to 0.4.

An overview of the changes:

- Call `cargo metadata` only once with `--no-deps`. Optional dependencies are filtered in `in_tree_crates` (handling `profiler_builtins` and `rustc_codegen_llvm` which are driven by the config).
- Remove a duplicate call to `metadata::build` when using `-v`. I'm not sure why it was there, it looks like a mistake or vestigial from previous behavior.
- Remove check for `_shim`, I believe all the `_shim` crates are now gone.
- Remove check for `rustc_` and `*san` for `test::Crate::should_run`, these are no longer dependencies in the `test` tree.
- Use relative paths in `./x.py test -h -v` output.
- Some code cleanup (remove unnecessary `find_compiler_crates`, etc.).
- Show suite paths (`src/test/ui/...`) in `./x.py test -h -v` output.
- Some doc comments.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2020
Rollup of 10 pull requests

Successful merges:

 - rust-lang#72280 (Fix up autoderef when reborrowing)
 - rust-lang#72785 (linker: MSVC supports linking static libraries as a whole archive)
 - rust-lang#73011 (first stage of implementing LLVM code coverage)
 - rust-lang#73044 (compiletest: Add directives to detect sanitizer support)
 - rust-lang#73054 (memory access sanity checks: abort instead of panic)
 - rust-lang#73136 (Change how compiler-builtins gets many CGUs)
 - rust-lang#73280 (Add E0763)
 - rust-lang#73317 (bootstrap: read config from $RUST_BOOTSTRAP_CONFIG)
 - rust-lang#73350 (bootstrap/install.rs: support a nonexistent `prefix` in `x.py install`)
 - rust-lang#73352 (Speed up bootstrap a little.)

Failed merges:

r? @ghost
@bors bors merged commit 61c8925 into rust-lang:master Jun 19, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 8, 2020
…r=Mark-Simulacrum

Fix x.py test for librustc crates.

rust-lang#73352 introduced a bug where `x.py test src/librustc_ast` would fail to actually run the tests. The issue is that `krate` and `all_krates` were changed to return relative paths. This caused the code to do a test of "relative_path ends with absolute path" which is always false.  The solution is to swap that around.

The change to `Crate` isn't necessary, it just simplifies the code and makes it uniform with `CrateLibrustc`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2020
…Mark-Simulacrum

Fix x.py test for librustc crates.

rust-lang#73352 introduced a bug where `x.py test src/librustc_ast` would fail to actually run the tests. The issue is that `krate` and `all_krates` were changed to return relative paths. This caused the code to do a test of "relative_path ends with absolute path" which is always false.  The solution is to swap that around.

The change to `Crate` isn't necessary, it just simplifies the code and makes it uniform with `CrateLibrustc`.
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants