Skip to content

Commit 09e36ce

Browse files
committed
[compiletest] Parallelize test discovery
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.
1 parent 6bc57c6 commit 09e36ce

File tree

3 files changed

+61
-38
lines changed

3 files changed

+61
-38
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,7 @@ dependencies = [
729729
"libc",
730730
"miow",
731731
"miropt-test-tools",
732+
"rayon",
732733
"regex",
733734
"rustfix",
734735
"semver",

src/tools/compiletest/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ glob = "0.3.0"
1818
home = "0.5.5"
1919
indexmap = "2.0.0"
2020
miropt-test-tools = { path = "../miropt-test-tools" }
21+
rayon = "1.10.0"
2122
regex = "1.0"
2223
rustfix = "0.8.1"
2324
semver = { version = "1.0.23", features = ["serde"] }

src/tools/compiletest/src/lib.rs

+59-38
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use std::{env, fs, vec};
3333
use build_helper::git::{get_git_modified_files, get_git_untracked_files};
3434
use camino::{Utf8Path, Utf8PathBuf};
3535
use getopts::Options;
36+
use rayon::iter::{ParallelBridge, ParallelIterator};
3637
use tracing::*;
3738
use walkdir::WalkDir;
3839

@@ -640,6 +641,18 @@ struct TestCollector {
640641
poisoned: bool,
641642
}
642643

644+
impl TestCollector {
645+
fn new() -> Self {
646+
TestCollector { tests: vec![], found_path_stems: HashSet::new(), poisoned: false }
647+
}
648+
649+
fn merge(&mut self, mut other: Self) {
650+
self.tests.append(&mut other.tests);
651+
self.found_path_stems.extend(other.found_path_stems);
652+
self.poisoned |= other.poisoned;
653+
}
654+
}
655+
643656
/// Creates test structures for every test/revision in the test suite directory.
644657
///
645658
/// This always inspects _all_ test files in the suite (e.g. all 17k+ ui tests),
@@ -658,10 +671,7 @@ pub(crate) fn collect_and_make_tests(config: Arc<Config>) -> Vec<CollectedTest>
658671
let cache = HeadersCache::load(&config);
659672

660673
let cx = TestCollectorCx { config, cache, common_inputs_stamp, modified_tests };
661-
let mut collector =
662-
TestCollector { tests: vec![], found_path_stems: HashSet::new(), poisoned: false };
663-
664-
collect_tests_from_dir(&cx, &mut collector, &cx.config.src_test_suite_root, Utf8Path::new(""))
674+
let collector = collect_tests_from_dir(&cx, &cx.config.src_test_suite_root, Utf8Path::new(""))
665675
.unwrap_or_else(|reason| {
666676
panic!("Could not read tests from {}: {reason}", cx.config.src_test_suite_root)
667677
});
@@ -767,25 +777,25 @@ fn modified_tests(config: &Config, dir: &Utf8Path) -> Result<Vec<Utf8PathBuf>, S
767777
/// that will be handed over to libtest.
768778
fn collect_tests_from_dir(
769779
cx: &TestCollectorCx,
770-
collector: &mut TestCollector,
771780
dir: &Utf8Path,
772781
relative_dir_path: &Utf8Path,
773-
) -> io::Result<()> {
782+
) -> io::Result<TestCollector> {
774783
// Ignore directories that contain a file named `compiletest-ignore-dir`.
775784
if dir.join("compiletest-ignore-dir").exists() {
776-
return Ok(());
785+
return Ok(TestCollector::new());
777786
}
778787

779788
// For run-make tests, a "test file" is actually a directory that contains an `rmake.rs`.
780789
if cx.config.mode == Mode::RunMake {
790+
let mut collector = TestCollector::new();
781791
if dir.join("rmake.rs").exists() {
782792
let paths = TestPaths {
783793
file: dir.to_path_buf(),
784794
relative_dir: relative_dir_path.parent().unwrap().to_path_buf(),
785795
};
786-
make_test(cx, collector, &paths);
796+
make_test(cx, &mut collector, &paths);
787797
// This directory is a test, so don't try to find other tests inside it.
788-
return Ok(());
798+
return Ok(collector);
789799
}
790800
}
791801

@@ -802,36 +812,47 @@ fn collect_tests_from_dir(
802812
// subdirectories we find, except for `auxiliary` directories.
803813
// FIXME: this walks full tests tree, even if we have something to ignore
804814
// use walkdir/ignore like in tidy?
805-
for file in fs::read_dir(dir.as_std_path())? {
806-
let file = file?;
807-
let file_path = Utf8PathBuf::try_from(file.path()).unwrap();
808-
let file_name = file_path.file_name().unwrap();
809-
810-
if is_test(file_name)
811-
&& (!cx.config.only_modified || cx.modified_tests.contains(&file_path))
812-
{
813-
// We found a test file, so create the corresponding libtest structures.
814-
debug!(%file_path, "found test file");
815-
816-
// Record the stem of the test file, to check for overlaps later.
817-
let rel_test_path = relative_dir_path.join(file_path.file_stem().unwrap());
818-
collector.found_path_stems.insert(rel_test_path);
819-
820-
let paths =
821-
TestPaths { file: file_path, relative_dir: relative_dir_path.to_path_buf() };
822-
make_test(cx, collector, &paths);
823-
} else if file_path.is_dir() {
824-
// Recurse to find more tests in a subdirectory.
825-
let relative_file_path = relative_dir_path.join(file_name);
826-
if file_name != "auxiliary" {
827-
debug!(%file_path, "found directory");
828-
collect_tests_from_dir(cx, collector, &file_path, &relative_file_path)?;
815+
fs::read_dir(dir.as_std_path())?
816+
.par_bridge()
817+
.map(|file| {
818+
let mut collector = TestCollector::new();
819+
let file = file?;
820+
let file_path = Utf8PathBuf::try_from(file.path()).unwrap();
821+
let file_name = file_path.file_name().unwrap();
822+
823+
if is_test(file_name)
824+
&& (!cx.config.only_modified || cx.modified_tests.contains(&file_path))
825+
{
826+
// We found a test file, so create the corresponding libtest structures.
827+
debug!(%file_path, "found test file");
828+
829+
// Record the stem of the test file, to check for overlaps later.
830+
let rel_test_path = relative_dir_path.join(file_path.file_stem().unwrap());
831+
collector.found_path_stems.insert(rel_test_path);
832+
833+
let paths =
834+
TestPaths { file: file_path, relative_dir: relative_dir_path.to_path_buf() };
835+
make_test(cx, &mut collector, &paths);
836+
} else if file_path.is_dir() {
837+
// Recurse to find more tests in a subdirectory.
838+
let relative_file_path = relative_dir_path.join(file_name);
839+
if file_name != "auxiliary" {
840+
debug!(%file_path, "found directory");
841+
collector.merge(collect_tests_from_dir(cx, &file_path, &relative_file_path)?);
842+
}
843+
} else {
844+
debug!(%file_path, "found other file/directory");
829845
}
830-
} else {
831-
debug!(%file_path, "found other file/directory");
832-
}
833-
}
834-
Ok(())
846+
Ok(collector)
847+
})
848+
.reduce(
849+
|| Ok(TestCollector::new()),
850+
|a, b| {
851+
let mut a = a?;
852+
a.merge(b?);
853+
Ok(a)
854+
},
855+
)
835856
}
836857

837858
/// Returns true if `file_name` looks like a proper test file name.

0 commit comments

Comments
 (0)