Skip to content

Commit 740b447

Browse files
Mark-SimulacrumMark-Simulacrum
authored
Merge pull request #692 from Mark-Simulacrum/cut-duration
Reduce run durations
2 parents 2a965e2 + 61bad5f commit 740b447

File tree

13 files changed

+197
-67
lines changed

13 files changed

+197
-67
lines changed

‎ci/check-benchmarks.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ set -x;
55
bash -c "while true; do sleep 30; echo \$(date) - running ...; done" &
66
PING_LOOP_PID=$!
77
trap - ERR
8-
RUST_BACKTRACE=1 RUST_LOG=collector=debug,rust_sysroot=debug \
8+
RUST_BACKTRACE=1 RUST_LOG=collector_raw_cargo=trace,collector=debug,rust_sysroot=debug \
99
cargo run -p collector --bin collector -- \
1010
bench_test $BENCH_TEST_OPTS
1111
code=$?

‎collector/benchmarks/cranelift-codegen/perf-config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
{
2-
"cargo_toml": "cranelift-codegen/Cargo.toml"
2+
"cargo_toml": "cranelift-codegen/Cargo.toml",
3+
"touch_file": "cranelift-codegen/src/lib.rs"
34
}

‎collector/benchmarks/script-servo-2/components/selectors/build.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ fn main() {
2424
set.build(),
2525
)
2626
.unwrap();
27+
println!("cargo:rerun-if-changed=build.rs");
2728
}
2829

2930
/// <https://html.spec.whatwg.org/multipage/#selectors>

‎collector/benchmarks/script-servo-2/perf-config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"cargo_opts": "--no-default-features",
33
"cargo_toml": "components/script/Cargo.toml",
4-
"runs": 1
4+
"runs": 1,
5+
"touch_file": "components/script/lib.rs"
56
}

‎collector/benchmarks/serde/perf-config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
{
2-
"cargo_toml": "serde/Cargo.toml"
2+
"cargo_toml": "serde/Cargo.toml",
3+
"touch_file": "serde/src/lib.rs"
34
}

‎collector/benchmarks/style-servo/components/selectors/build.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,18 @@ use std::io::{BufWriter, Write};
1010
use std::path::Path;
1111

1212
fn main() {
13-
let path = Path::new(&env::var("OUT_DIR").unwrap())
14-
.join("ascii_case_insensitive_html_attributes.rs");
13+
let path =
14+
Path::new(&env::var("OUT_DIR").unwrap()).join("ascii_case_insensitive_html_attributes.rs");
1515
let mut file = BufWriter::new(File::create(&path).unwrap());
1616

17-
write!(&mut file, "{{ static SET: ::phf::Set<&'static str> = ",
18-
).unwrap();
17+
write!(&mut file, "{{ static SET: ::phf::Set<&'static str> = ",).unwrap();
1918
let mut set = phf_codegen::Set::new();
2019
for name in ASCII_CASE_INSENSITIVE_HTML_ATTRIBUTES.split_whitespace() {
2120
set.entry(name);
2221
}
2322
set.build(&mut file).unwrap();
2423
write!(&mut file, "; &SET }}").unwrap();
24+
println!("cargo:rerun-if-changed=build.rs");
2525
}
2626

2727
/// https://html.spec.whatwg.org/multipage/#selectors

‎collector/benchmarks/style-servo/perf-config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@
33
"cargo_rustc_opts": "--cap-lints=warn",
44
"cargo_toml": "components/style/Cargo.toml",
55
"runs": 1,
6-
"supports_stable": true
6+
"supports_stable": true,
7+
"touch_file": "components/style/lib.rs"
78
}

‎collector/benchmarks/webrender-wrench/perf-config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{
22
"cargo_toml": "wrench/Cargo.toml",
3-
"runs": 1
3+
"runs": 1,
4+
"touch_file": "wrench/src/main.rs"
45
}

‎collector/benchmarks/webrender/perf-config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{
22
"cargo_toml": "webrender/Cargo.toml",
3-
"runs": 1
3+
"runs": 1,
4+
"touch_file": "webrender/src/lib.rs"
45
}

‎collector/src/execute.rs

Lines changed: 64 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ use std::time::Duration;
1919
use tempfile::TempDir;
2020
use tokio::runtime::Runtime;
2121

22+
fn touch(root: &Path, path: &Path) -> anyhow::Result<()> {
23+
let mut cmd = Command::new("touch");
24+
cmd.current_dir(root).arg(path);
25+
command_output(&mut cmd).with_context(|| format!("touching {:?} in {:?}", path, root))?;
26+
Ok(())
27+
}
28+
2229
fn touch_all(path: &Path) -> anyhow::Result<()> {
2330
let mut cmd = Command::new("bash");
2431
cmd.current_dir(path)
@@ -50,6 +57,13 @@ struct BenchmarkConfig {
5057
runs: usize,
5158
#[serde(default)]
5259
supports_stable: bool,
60+
61+
/// The file that should be touched to ensure cargo re-checks the leaf crate
62+
/// we're interested in. Likely, something similar to `src/lib.rs`. The
63+
/// default if this is not present is to touch all .rs files in the
64+
/// directory that `Cargo.toml` is in.
65+
#[serde(default)]
66+
touch_file: Option<String>,
5367
}
5468

5569
impl Default for BenchmarkConfig {
@@ -61,6 +75,7 @@ impl Default for BenchmarkConfig {
6175
disabled: false,
6276
runs: default_runs(),
6377
supports_stable: false,
78+
touch_file: None,
6479
}
6580
}
6681
}
@@ -191,6 +206,7 @@ struct CargoProcess<'a> {
191206
manifest_path: String,
192207
cargo_args: Vec<String>,
193208
rustc_args: Vec<String>,
209+
touch_file: Option<String>,
194210
}
195211

196212
impl<'a> CargoProcess<'a> {
@@ -301,13 +317,17 @@ impl<'a> CargoProcess<'a> {
301317
// benchmarking, so as to not refresh dependencies, which may be
302318
// in-tree (e.g., in the case of the servo crates there are a lot of
303319
// other components).
304-
touch_all(
305-
&self.cwd.join(
306-
Path::new(&self.manifest_path)
307-
.parent()
308-
.expect("manifest has parent"),
309-
),
310-
)?;
320+
if let Some(file) = &self.touch_file {
321+
touch(&self.cwd, Path::new(&file))?;
322+
} else {
323+
touch_all(
324+
&self.cwd.join(
325+
Path::new(&self.manifest_path)
326+
.parent()
327+
.expect("manifest has parent"),
328+
),
329+
)?;
330+
}
311331

312332
let output = command_output(&mut cmd)?;
313333
if let Some((ref mut processor, run_kind, run_kind_str, patch)) = self.processor_etc {
@@ -396,10 +416,6 @@ pub trait Processor {
396416
fn finished_first_collection(&mut self) -> bool {
397417
false
398418
}
399-
400-
/// Called when all the runs of a benchmark for a particular `BuildKind`
401-
/// and iteration have been completed. Can be used to process/reset accumulated state.
402-
fn finish_build_kind(&mut self, _build_kind: BuildKind) {}
403419
}
404420

405421
pub struct MeasureProcessor<'a> {
@@ -567,10 +583,6 @@ impl<'a> Processor for MeasureProcessor<'a> {
567583
}
568584
}
569585
}
570-
571-
fn finish_build_kind(&mut self, _: BuildKind) {
572-
// do nothing
573-
}
574586
}
575587

576588
pub struct ProfileProcessor<'a> {
@@ -916,6 +928,7 @@ impl Benchmark {
916928
.split_whitespace()
917929
.map(String::from)
918930
.collect(),
931+
touch_file: self.config.touch_file.clone(),
919932
}
920933
}
921934

@@ -976,42 +989,48 @@ impl Benchmark {
976989
.run_rustc()?;
977990
}
978991

979-
// An incremental build from scratch (slowest incremental case).
980-
// This is required for any subsequent incremental builds.
981-
if run_kinds.contains(&RunKind::IncrFull)
982-
|| run_kinds.contains(&RunKind::IncrUnchanged)
983-
|| run_kinds.contains(&RunKind::IncrPatched)
984-
{
985-
self.mk_cargo_process(compiler, cwd, build_kind)
986-
.incremental(true)
987-
.processor(processor, RunKind::IncrFull, "IncrFull", None)
988-
.run_rustc()?;
989-
}
990-
991-
// An incremental build with no changes (fastest incremental case).
992-
if run_kinds.contains(&RunKind::IncrUnchanged) {
993-
self.mk_cargo_process(compiler, cwd, build_kind)
994-
.incremental(true)
995-
.processor(processor, RunKind::IncrUnchanged, "IncrUnchanged", None)
996-
.run_rustc()?;
997-
}
998-
999-
if run_kinds.contains(&RunKind::IncrPatched) {
1000-
for (i, patch) in self.patches.iter().enumerate() {
1001-
log::debug!("applying patch {}", patch.name);
1002-
patch.apply(cwd).map_err(|s| anyhow::anyhow!("{}", s))?;
992+
// Rustdoc does not support incremental compilation
993+
if build_kind != BuildKind::Doc {
994+
// An incremental build from scratch (slowest incremental case).
995+
// This is required for any subsequent incremental builds.
996+
if run_kinds.contains(&RunKind::IncrFull)
997+
|| run_kinds.contains(&RunKind::IncrUnchanged)
998+
|| run_kinds.contains(&RunKind::IncrPatched)
999+
{
1000+
self.mk_cargo_process(compiler, cwd, build_kind)
1001+
.incremental(true)
1002+
.processor(processor, RunKind::IncrFull, "IncrFull", None)
1003+
.run_rustc()?;
1004+
}
10031005

1004-
// An incremental build with some changes (realistic
1005-
// incremental case).
1006-
let run_kind_str = format!("IncrPatched{}", i);
1006+
// An incremental build with no changes (fastest incremental case).
1007+
if run_kinds.contains(&RunKind::IncrUnchanged) {
10071008
self.mk_cargo_process(compiler, cwd, build_kind)
10081009
.incremental(true)
1009-
.processor(processor, RunKind::IncrPatched, &run_kind_str, Some(&patch))
1010+
.processor(processor, RunKind::IncrUnchanged, "IncrUnchanged", None)
10101011
.run_rustc()?;
10111012
}
1012-
}
10131013

1014-
processor.finish_build_kind(build_kind);
1014+
if run_kinds.contains(&RunKind::IncrPatched) {
1015+
for (i, patch) in self.patches.iter().enumerate() {
1016+
log::debug!("applying patch {}", patch.name);
1017+
patch.apply(cwd).map_err(|s| anyhow::anyhow!("{}", s))?;
1018+
1019+
// An incremental build with some changes (realistic
1020+
// incremental case).
1021+
let run_kind_str = format!("IncrPatched{}", i);
1022+
self.mk_cargo_process(compiler, cwd, build_kind)
1023+
.incremental(true)
1024+
.processor(
1025+
processor,
1026+
RunKind::IncrPatched,
1027+
&run_kind_str,
1028+
Some(&patch),
1029+
)
1030+
.run_rustc()?;
1031+
}
1032+
}
1033+
}
10151034
}
10161035
}
10171036

‎collector/src/lib.rs

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ use std::fmt;
66
use std::process::{self, Command};
77

88
pub mod api;
9+
mod read2;
910
pub mod self_profile;
1011

12+
use process::Stdio;
1113
pub use self_profile::{QueryData, SelfProfile};
1214

1315
#[derive(Debug, Copy, Clone, PartialEq, PartialOrd, Deserialize)]
@@ -135,20 +137,47 @@ pub fn run_command(cmd: &mut Command) -> anyhow::Result<()> {
135137

136138
pub fn command_output(cmd: &mut Command) -> anyhow::Result<process::Output> {
137139
log::trace!("running: {:?}", cmd);
138-
let output = cmd.output()?;
139-
if !output.status.success() {
140+
let mut child = cmd.stdout(Stdio::piped()).stderr(Stdio::piped()).spawn()?;
141+
142+
let mut stdout = Vec::new();
143+
let mut stderr = Vec::new();
144+
let mut stdout_writer = std::io::LineWriter::new(std::io::stdout());
145+
let mut stderr_writer = std::io::LineWriter::new(std::io::stderr());
146+
read2::read2(
147+
child.stdout.take().unwrap(),
148+
child.stderr.take().unwrap(),
149+
&mut |is_stdout, buffer, _is_done| {
150+
// Send output if trace logging is enabled
151+
if log::log_enabled!(target: "collector_raw_cargo", log::Level::Trace) {
152+
use std::io::Write;
153+
if is_stdout {
154+
stdout_writer.write_all(&buffer[stdout.len()..]).unwrap();
155+
} else {
156+
stderr_writer.write_all(&buffer[stderr.len()..]).unwrap();
157+
}
158+
}
159+
if is_stdout {
160+
stdout = buffer.clone();
161+
} else {
162+
stderr = buffer.clone();
163+
}
164+
},
165+
)?;
166+
167+
let status = child.wait()?;
168+
if !status.success() {
140169
return Err(anyhow::anyhow!(
141170
"expected success, got {}\n\nstderr={}\n\n stdout={}",
142-
output.status,
143-
String::from_utf8_lossy(&output.stderr),
144-
String::from_utf8_lossy(&output.stdout)
171+
status,
172+
String::from_utf8_lossy(&stderr),
173+
String::from_utf8_lossy(&stdout)
145174
));
146-
} else {
147-
// log::trace!(
148-
// "stderr={}\n\nstdout={}",
149-
// String::from_utf8_lossy(&output.stderr),
150-
// String::from_utf8_lossy(&output.stdout),
151-
// );
152175
}
176+
177+
let output = process::Output {
178+
status,
179+
stdout: stdout,
180+
stderr: stderr,
181+
};
153182
Ok(output)
154183
}

0 commit comments

Comments
 (0)