Skip to content

Commit 9fb1bd9

Browse files
authored
Rollup merge of rust-lang#81833 - the8472:parallel-bootstrap-rustfmt, r=Mark-Simulacrum
parallelize x.py test tidy Running tidy on individual commits when rewriting git history was somewhat of an annoyance, so I have parallelized it a bit. running `time ./x.py test tidy` with warm IO caches: old: ``` real 0m11.123s user 0m14.495s sys 0m5.227s ``` new: ``` real 0m1.834s user 0m13.545s sys 0m3.094s ``` There's further room for improvement (<0.9s should be feasible) but that would require bigger changes.
2 parents db384eb + c071970 commit 9fb1bd9

File tree

1 file changed

+63
-18
lines changed

1 file changed

+63
-18
lines changed

src/bootstrap/format.rs

+63-18
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
use crate::Build;
44
use build_helper::{output, t};
55
use ignore::WalkBuilder;
6-
use std::path::Path;
6+
use std::collections::VecDeque;
7+
use std::path::{Path, PathBuf};
78
use std::process::{Command, Stdio};
9+
use std::sync::mpsc::SyncSender;
810

9-
fn rustfmt(src: &Path, rustfmt: &Path, path: &Path, check: bool) {
11+
fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl FnMut() {
1012
let mut cmd = Command::new(&rustfmt);
1113
// avoid the submodule config paths from coming into play,
1214
// we only allow a single global config for the workspace for now
@@ -17,17 +19,21 @@ fn rustfmt(src: &Path, rustfmt: &Path, path: &Path, check: bool) {
1719
if check {
1820
cmd.arg("--check");
1921
}
20-
cmd.arg(&path);
22+
cmd.args(paths);
2123
let cmd_debug = format!("{:?}", cmd);
22-
let status = cmd.status().expect("executing rustfmt");
23-
if !status.success() {
24-
eprintln!(
25-
"Running `{}` failed.\nIf you're running `tidy`, \
26-
try again with `--bless`. Or, if you just want to format \
27-
code, run `./x.py fmt` instead.",
28-
cmd_debug,
29-
);
30-
std::process::exit(1);
24+
let mut cmd = cmd.spawn().expect("running rustfmt");
25+
// poor man's async: return a closure that'll wait for rustfmt's completion
26+
move || {
27+
let status = cmd.wait().unwrap();
28+
if !status.success() {
29+
eprintln!(
30+
"Running `{}` failed.\nIf you're running `tidy`, \
31+
try again with `--bless`. Or, if you just want to format \
32+
code, run `./x.py fmt` instead.",
33+
cmd_debug,
34+
);
35+
std::process::exit(1);
36+
}
3137
}
3238
}
3339

@@ -101,19 +107,58 @@ pub fn format(build: &Build, check: bool) {
101107
}
102108
let ignore_fmt = ignore_fmt.build().unwrap();
103109

104-
let rustfmt_path = build.config.initial_rustfmt.as_ref().unwrap_or_else(|| {
105-
eprintln!("./x.py fmt is not supported on this channel");
106-
std::process::exit(1);
110+
let rustfmt_path = build
111+
.config
112+
.initial_rustfmt
113+
.as_ref()
114+
.unwrap_or_else(|| {
115+
eprintln!("./x.py fmt is not supported on this channel");
116+
std::process::exit(1);
117+
})
118+
.to_path_buf();
119+
let src = build.src.clone();
120+
let (tx, rx): (SyncSender<PathBuf>, _) = std::sync::mpsc::sync_channel(128);
121+
let walker =
122+
WalkBuilder::new(src.clone()).types(matcher).overrides(ignore_fmt).build_parallel();
123+
124+
// there is a lot of blocking involved in spawning a child process and reading files to format.
125+
// spawn more processes than available concurrency to keep the CPU busy
126+
let max_processes = build.jobs() as usize * 2;
127+
128+
// spawn child processes on a separate thread so we can batch entries we have received from ignore
129+
let thread = std::thread::spawn(move || {
130+
let mut children = VecDeque::new();
131+
while let Ok(path) = rx.recv() {
132+
// try getting a few more paths from the channel to amortize the overhead of spawning processes
133+
let paths: Vec<_> = rx.try_iter().take(7).chain(std::iter::once(path)).collect();
134+
135+
let child = rustfmt(&src, &rustfmt_path, paths.as_slice(), check);
136+
children.push_back(child);
137+
138+
if children.len() >= max_processes {
139+
// await oldest child
140+
children.pop_front().unwrap()();
141+
}
142+
}
143+
144+
// await remaining children
145+
for mut child in children {
146+
child();
147+
}
107148
});
108-
let src = &build.src;
109-
let walker = WalkBuilder::new(src).types(matcher).overrides(ignore_fmt).build_parallel();
149+
110150
walker.run(|| {
151+
let tx = tx.clone();
111152
Box::new(move |entry| {
112153
let entry = t!(entry);
113154
if entry.file_type().map_or(false, |t| t.is_file()) {
114-
rustfmt(src, &rustfmt_path, &entry.path(), check);
155+
t!(tx.send(entry.into_path()));
115156
}
116157
ignore::WalkState::Continue
117158
})
118159
});
160+
161+
drop(tx);
162+
163+
thread.join().unwrap();
119164
}

0 commit comments

Comments
 (0)