Skip to content

[compiletest] Parallelize test discovery #140177

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

Merged
merged 2 commits into from
Apr 26, 2025
Merged

Conversation

tmandry
Copy link
Member

@tmandry tmandry commented Apr 22, 2025

Certain filesystems are slow to service individual read requests, but can service many in parallel. This change brings down the time to run a single cached test on one of those filesystems from 40s to about 8s.

@rustbot
Copy link
Collaborator

rustbot commented Apr 22, 2025

r? @wesleywiser

rustbot has assigned @wesleywiser.
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 A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc 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 Apr 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 22, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rustbot

This comment has been minimized.

Certain filesystems for large monorepos are slow to service individual
read requests, but can service many in parallel. This change brings down
the time to run a single cached test on one of those filesystems from
40s to about 8s.
@jieyouxu jieyouxu assigned jieyouxu and unassigned wesleywiser Apr 23, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks! This is quite a significant improvement even in my local testing. It turns out

Certain filesystems

can even include ReFS on a local Windows dev drive.

Test collection time diff

Measured against 1a5bf12 + naive time recording around top-level collect_tests_from_dir.

x86_64-unknown-linux-gnu (WSL)

State collect_tests_from_dir duration on tests/ui
Without this PR 362.296834ms
With this PR 248.394981ms

x86_64-pc-windows-msvc (PowerShell, native)

State collect_tests_from_dir duration on tests/ui
Without this PR 2.5187741s
With this PR 507.9907ms

Additionally, I ran ./x test ui and ./x test run-make to double-check if the number of runned tests and test outcomes look "expected", which they did.

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 23, 2025

📌 Commit 09e36ce has been approved by jieyouxu

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 Apr 23, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 24, 2025
[compiletest] Parallelize test discovery

Certain filesystems are slow to service individual read requests, but can service many in parallel. This change brings down the time to run a single cached test on one of those filesystems from 40s to about 8s.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 24, 2025
Rollup of 23 pull requests

Successful merges:

 - rust-lang#139261 (mitigate MSVC alignment issue on x86-32)
 - rust-lang#139307 (std: Add performance warnings to HashMap::get_disjoint_mut)
 - rust-lang#139700 (Autodiff flags)
 - rust-lang#139752 (set subsections_via_symbols for ld64 helper sections)
 - rust-lang#139809 (Don't warn about `v128` in wasm ABI transition)
 - rust-lang#139852 (StableMIR: Implement `CompilerInterface`)
 - rust-lang#139945 (Extend HIR to track the source and syntax of a lifetime)
 - rust-lang#140028 (`deref_patterns`: support string and byte string literals in explicit `deref!("...")` patterns)
 - rust-lang#140139 (rustc_target: Adjust RISC-V feature implication)
 - rust-lang#140143 (Move `sys::pal::os::Env` into `sys::env`)
 - rust-lang#140148 (CI: use aws codebuild for job dist-arm-linux)
 - rust-lang#140150 (fix MAX_EXP and MIN_EXP docs)
 - rust-lang#140172 (Make algebraic functions into `const fn` items.)
 - rust-lang#140177 ([compiletest] Parallelize test discovery)
 - rust-lang#140181 (Remove `synstructure::Structure::underscore_const` calls.)
 - rust-lang#140184 (Update doc of cygwin target)
 - rust-lang#140186 (Rename `compute_x` methods)
 - rust-lang#140187 ([AIX] Handle AIX dynamic library extensions within c-link-to-rust-dylib run-make test)
 - rust-lang#140191 (Remove git repository from git config)
 - rust-lang#140194 (minicore: Have `//@ add-core-stubs` also imply `-Cforce-unwind-tables=yes`)
 - rust-lang#140195 (triagebot: label minicore changes w/ `A-test-infra-minicore` and ping jieyouxu on changes)
 - rust-lang#140202 (Make #![feature(let_chains)] bootstrap conditional in compiler/)
 - rust-lang#140214 (Remove comment about handling non-global where bounds with corresponding projection)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 24, 2025
[compiletest] Parallelize test discovery

Certain filesystems are slow to service individual read requests, but can service many in parallel. This change brings down the time to run a single cached test on one of those filesystems from 40s to about 8s.
@jieyouxu
Copy link
Member

Wait sorry this shouldn't have been rollup=always.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 24, 2025
@jieyouxu
Copy link
Member

@tmandry could you introduce a synthetic compiler tree change, like maybe a temporary comment in rustc/Cargo.toml, to inhibit download-rustc from kicking in? I'd like to double-check through post-merge test metrics that we don't have unexpected test changes. I'll follow-up to revert that comment addition. Like include a

# FIXME(jieyouxu): remove this comment after #140177 lands

(Maybe tool changes should inhibit download-rustc in CI, but that's a separate topic)

@jieyouxu
Copy link
Member

For post-merge test metrics,
@bors rollup=never

@jieyouxu
Copy link
Member

Please feel free to r=me after adding a synthetic compiler/ tree change, I'll revert that change after this PR merges if there are no unexpected test changes.

@rustbot
Copy link
Collaborator

rustbot commented Apr 24, 2025

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@Zalathar
Copy link
Contributor

I've pushed a trivial compiler style fix to inhibit download-rustc. No need to revert it afterwards.

@bors r=jieyouxu

@bors
Copy link
Collaborator

bors commented Apr 24, 2025

📌 Commit f673c9b has been approved by jieyouxu

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2025
@bors
Copy link
Collaborator

bors commented Apr 26, 2025

⌛ Testing commit f673c9b with merge d3508a8...

@bors
Copy link
Collaborator

bors commented Apr 26, 2025

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing d3508a8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 26, 2025
@bors bors merged commit d3508a8 into rust-lang:master Apr 26, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 26, 2025
Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 555e1d0 (parent) -> d3508a8 (this PR)

Test differences

No test diffs found

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard d3508a8ad0163fab0c9b2188b3adf43c87200788 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-aarch64-linux: 7658.2s -> 5255.6s (-31.4%)
  2. dist-apple-various: 6529.6s -> 8329.2s (27.6%)
  3. x86_64-apple-1: 8007.5s -> 9612.9s (20.0%)
  4. dist-x86_64-apple: 9499.8s -> 10367.2s (9.1%)
  5. dist-aarch64-apple: 5547.4s -> 5104.4s (-8.0%)
  6. dist-i686-mingw: 8078.5s -> 8662.1s (7.2%)
  7. dist-i686-msvc: 6779.1s -> 7144.3s (5.4%)
  8. aarch64-gnu: 6685.8s -> 6362.0s (-4.8%)
  9. x86_64-msvc-2: 6838.2s -> 7130.6s (4.3%)
  10. x86_64-rust-for-linux: 2740.5s -> 2624.0s (-4.3%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d3508a8): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 0.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
3.0% [3.0%, 3.0%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.7% [-0.4%, 3.0%] 3

Cycles

Results (primary -0.6%, secondary 3.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.0% [3.0%, 3.0%] 1
Improvements ✅
(primary)
-0.6% [-1.0%, -0.5%] 5
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.6% [-1.0%, -0.5%] 5

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 776.189s -> 775.41s (-0.10%)
Artifact size: 365.11 MiB -> 365.09 MiB (-0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. 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.

7 participants