add a cache for discovered workspace roots

[Muscraft]

History

@ehuss noticed that workspace inheritance caused a significant increase in startup times when using workspace inheritance. This brought up the creation of #10747.

When using a similar test setup to the original I got

Benchmark 1: cd rust; ../../../target/release/cargo metadata
  Time (mean ± σ):     149.4 ms ±   3.8 ms    [User: 105.9 ms, System: 31.7 ms]
  Range (min … max):   144.2 ms … 162.2 ms    19 runs
 
Benchmark 2: cd rust-ws-inherit; ../../../target/release/cargo metadata
  Time (mean ± σ):     191.6 ms ±   1.4 ms    [User: 145.9 ms, System: 34.2 ms]
  Range (min … max):   188.8 ms … 193.9 ms    15 runs

This showed a large increase in time per cargo command when using workspace inheritance.

During the investigation of this issue, other performance concerns were found and addressed. This resulted in a drop in time across the board but heavily favored workspace inheritance.

Benchmark 1: cd rust; ../../../target/release/cargo metadata
  Time (mean ± σ):     139.3 ms ±   1.7 ms    [User: 99.8 ms, System: 29.4 ms]
  Range (min … max):   137.1 ms … 144.5 ms    20 runs
 
Benchmark 2: cd rust-ws-inherit; ../../../target/release/cargo metadata
  Time (mean ± σ):     161.7 ms ±   1.4 ms    [User: 120.4 ms, System: 31.2 ms]
  Range (min … max):   158.0 ms … 164.6 ms    18 runs

Performance after changes

hyperfine --warmup 10 "cd rust; ../../../target/release/cargo metadata" "cd rust-ws-inherit; ../../../target/release/cargo metadata" --runs 40

Benchmark 1: cd rust; ../../../target/release/cargo metadata
  Time (mean ± σ):     140.1 ms ±   1.5 ms    [User: 99.5 ms, System: 30.7 ms]
  Range (min … max):   137.4 ms … 144.0 ms    40 runs
 
Benchmark 2: cd rust-ws-inherit; ../../../target/release/cargo metadata
  Time (mean ± σ):     141.8 ms ±   1.6 ms    [User: 100.9 ms, System: 30.9 ms]
  Range (min … max):   138.4 ms … 145.4 ms    40 runs

New Benchmark
cargo bench -- workspace_initialization/rust

workspace_initialization/rust
    time:   [14.779 ms 14.880 ms 14.997 ms]
workspace_initialization/rust-ws-inherit
    time:   [16.235 ms 16.293 ms 16.359 ms]

Changes Made

• Pulled a commit from @ehuss that deduplicated finding a workspace root to make the changes easier
• Added a cache in Config to hold found WorkspaceRootConfigs
  • This makes it so manifests should only be parsed once
• Made WorkspaceRootConfig get added to the cache when parsing a manifest

Testing Steps

To check the new benchmark:

1. cd benches/benchsuite
2. cargo bench -- workspace_initialization/rust

Using hyperfine:

1. run cargo build --release
2. extract rust and rust-ws-inherit in benches/workspaces
3. cd benches/workspaces
4. Prime the target directory with a cache of rustc info. In rust and rust-ws-inherit, run: cargo +nightly c -p linkchecker. Otherwise it would be measuring rustc overhead.
5. run hyperfine --warmup 10 "cd rust; ../../../target/release/cargo metadata" "cd rust-ws-inherit; ../../../target/release/cargo metadata" --runs 40

closes #10747

[rust-highfive]

r? @ehuss

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

[weihanglo]

Seems reasonable to me. Just a few questions:

• Do we need to document how the result would be cached, as it affects some public API?
• Is there a chance that there is a long-running cargo API consumer getting outdated caches? Should be really rare edge case I guess.

[Muscraft]

- Do we need to document how the result would be cached, as it affects some public API?

I'm not sure that this affects any public API to my knowledge. It should just make it so we don't parse a workspace root twice if it has not been loaded yet into a list of workspace packages or when we do not have access to the list of workspace packages.

- Is there a chance that there is a long-running cargo API consumer getting outdated caches? Should be really rare edge case I guess.

This shouldn't be a problem unless there is an edit to a workspace root manifest while cargo is running. If there is an edit while cargo is running, I think it is best to have the edit appear when cargo is run next either way.

[weihanglo]

unless there is an edit to a workspace root manifest while cargo is running

That's exact what I meant. Anyway, I am going to merge this. The benchmark result on my machine is as great as yours. And we can always add docs later if needed.

[weihanglo]

@bors r+

Thanks for the PR!

[bors]

📌 Commit fd8bcf9 has been approved by weihanglo

[bors]

⌛ Testing commit fd8bcf9 with merge ca190ac...

[bors]

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing ca190ac to master...