Merge pull request #717 from Mark-Simulacrum/assert-single-build

Mark-Simulacrum · web-flow · commit f469ef7bd769 · 2020-07-26T16:52:35.000-04:00
Assert that we invoke rustc once per statistics run
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -34,37 +34,20 @@ jobs:
         if: github.ref == 'refs/heads/master' || github.ref == 'refs/heads/deploy'
 
   test_benchmarks:
+    strategy:
+      matrix:
+        BENCH_INCLUDE_EXCLUDE_OPTS: [
+          "--exclude script-servo,webrender-wrench,style-servo",
+          "--include webrender-wrench,style-servo",
+          "--include script-servo",
+        ]
+        BUILD_KINDS: [
+          "Check,Doc",
+          "Debug",
+          "Opt",
+        ]
     name: Test benchmarks
     runs-on: ubuntu-latest
-    steps:
-      - name: Checkout the source code
-        uses: actions/checkout@v2
-        with:
-          fetch-depth: 1
-
-      - name: Install latest beta
-        uses: actions-rs/toolchain@v1
-        with:
-          toolchain: beta
-          override: true
-
-      - name: Configure environment
-        run: |
-          sudo apt-get update
-          sudo apt-get install -y linux-tools-common linux-tools-generic linux-tools-`uname -r`
-          echo -1 | sudo tee /proc/sys/kernel/perf_event_paranoid
-
-      - name: Build collector
-        run: cargo build -p collector
-
-      - name: Check benchmarks
-        run: sh -x -c "ci/check-benchmarks.sh"
-        env:
-          BENCH_INCLUDE_EXCLUDE_OPTS: "--exclude script-servo"
-
-  test_script_servo:
-    name: Test benchmark script-servo
-    runs-on: ubuntu-latest
     steps:
       - name: Checkout the source code
         uses: actions/checkout@v2
@@ -84,15 +67,17 @@ jobs:
           echo -1 | sudo tee /proc/sys/kernel/perf_event_paranoid
 
       - name: Install servo dependencies
-        run: sudo apt-get install -y llvm-dev clang libx11-dev python2.7 autoconf2.13
+        run: sudo apt-get install -y llvm-dev clang libx11-dev python2.7 autoconf2.13 libjemalloc-dev
 
       - name: Build collector
         run: cargo build -p collector
 
       - name: Check benchmarks
         run: sh -x -c "ci/check-benchmarks.sh"
         env:
-          BENCH_INCLUDE_EXCLUDE_OPTS: "--include script-servo"
+          JEMALLOC_OVERRIDE: /usr/lib/x86_64-linux-gnu/libjemalloc.so
+          BENCH_INCLUDE_EXCLUDE_OPTS: ${{ matrix.BENCH_INCLUDE_EXCLUDE_OPTS }}
+          BUILD_KINDS: ${{ matrix.BUILD_KINDS }}
           SHELL: "/bin/bash"
 
   test_profiling:
diff --git a/ci/check-benchmarks.sh b/ci/check-benchmarks.sh
@@ -7,14 +7,17 @@ PING_LOOP_PID=$!
 trap 'kill $PING_LOOP_PID' ERR 1 2 3 6
 
 # Install a toolchain.
-RUST_BACKTRACE=1 RUST_LOG=collector_raw_cargo=trace,collector=debug,rust_sysroot=debug \
+RUST_BACKTRACE=1 RUST_LOG=raw_cargo_messages=trace,collector=debug,rust_sysroot=debug \
     bindir=`cargo run -p collector --bin collector install_next`
 
 # Do some benchmarking.
-RUST_BACKTRACE=1 RUST_LOG=collector_raw_cargo=trace,collector=debug,rust_sysroot=debug \
+RUST_BACKTRACE=1 \
+    RUST_LIB_BACKTRACE=0 \
+    CARGO_LOG=cargo::core::compiler::fingerprint=info \
+    RUST_LOG=raw_cargo_messages=trace,collector=debug,rust_sysroot=debug \
     cargo run -p collector --bin collector -- \
     bench_local $bindir/rustc Test \
-        --builds Check,Doc \
+        --builds $BUILD_KINDS \
         --cargo $bindir/cargo \
         --runs All \
         --rustdoc $bindir/rustdoc \
diff --git a/ci/check-profiling.sh b/ci/check-profiling.sh
@@ -10,11 +10,11 @@ PING_LOOP_PID=$!
 trap 'kill $PING_LOOP_PID' ERR 1 2 3 6
 
 # Install a toolchain.
-RUST_BACKTRACE=1 RUST_LOG=collector_raw_cargo=trace,collector=debug,rust_sysroot=debug \
+RUST_BACKTRACE=1 RUST_LOG=raw_cargo_messages=trace,collector=debug,rust_sysroot=debug \
     bindir=`cargo run -p collector --bin collector install_next`
 
 # Profile with eprintln.
-RUST_BACKTRACE=1 RUST_LOG=collector_raw_cargo=trace,collector=debug,rust_sysroot=debug \
+RUST_BACKTRACE=1 RUST_LOG=raw_cargo_messages=trace,collector=debug,rust_sysroot=debug \
     cargo run -p collector --bin collector -- \
     profile_local eprintln $bindir/rustc Test \
         --cargo $bindir/cargo \
@@ -29,7 +29,7 @@ test -f results/eprintln-Test-helloworld-Opt-Full
 test ! -e results/eprintln-Test-helloworld-Doc-Full
 
 # Profile with llvm-lines.
-RUST_BACKTRACE=1 RUST_LOG=collector_raw_cargo=trace,collector=debug,rust_sysroot=debug \
+RUST_BACKTRACE=1 RUST_LOG=raw_cargo_messages=trace,collector=debug,rust_sysroot=debug \
     cargo run -p collector --bin collector -- \
     profile_local llvm-lines $bindir/rustc Test \
         --builds Debug \
diff --git a/collector/benchmarks/cargo/perf-config.json b/collector/benchmarks/cargo/perf-config.json
@@ -1,4 +1,5 @@
 {
     "cargo_opts": "--lib",
-    "runs": 1
+    "runs": 1,
+    "touch_file": "src/cargo/lib.rs"
 }
diff --git a/collector/benchmarks/encoding/perf-config.json b/collector/benchmarks/encoding/perf-config.json
@@ -1,3 +1,4 @@
 {
-    "supports_stable": true
+    "supports_stable": true,
+    "touch_file": "src/lib.rs"
 }
diff --git a/collector/benchmarks/html5ever/perf-config.json b/collector/benchmarks/html5ever/perf-config.json
@@ -1,3 +1,4 @@
 {
-    "supports_stable": true
+    "supports_stable": true,
+    "touch_file": "src/lib.rs"
 }
diff --git a/collector/benchmarks/hyper-2/perf-config.json b/collector/benchmarks/hyper-2/perf-config.json
@@ -0,0 +1,3 @@
+{
+    "touch_file": "src/lib.rs"
+}
diff --git a/collector/benchmarks/native-tls-0.1.5/perf-config.json b/collector/benchmarks/native-tls-0.1.5/perf-config.json
@@ -0,0 +1,3 @@
+{
+    "touch_file": "src/lib.rs"
+}
diff --git a/collector/benchmarks/regex/perf-config.json b/collector/benchmarks/regex/perf-config.json
@@ -1,3 +1,4 @@
 {
-    "supports_stable": true
+    "supports_stable": true,
+    "touch_file": "src/lib.rs"
 }
diff --git a/collector/benchmarks/ripgrep/perf-config.json b/collector/benchmarks/ripgrep/perf-config.json
@@ -0,0 +1,3 @@
+{
+    "touch_file": "src/main.rs"
+}
diff --git a/collector/benchmarks/style-servo/components/style/build.rs b/collector/benchmarks/style-servo/components/style/build.rs
@@ -63,7 +63,6 @@ fn generate_properties() {
         let entry = entry.unwrap();
         match entry.path().extension().and_then(|e| e.to_str()) {
             Some("mako") | Some("rs") | Some("py") | Some("zip") => {
-                println!("cargo:rerun-if-changed={}", entry.path().display());
             }
             _ => {}
         }
@@ -85,7 +84,6 @@ fn generate_properties() {
 
 fn main() {
     println!("cargo:rerun-if-changed=build.rs");
-    println!("cargo:out_dir={}", env::var("OUT_DIR").unwrap());
     generate_properties();
     build_gecko::generate();
 }
diff --git a/collector/benchmarks/style-servo/components/style/build_gecko.rs b/collector/benchmarks/style-servo/components/style/build_gecko.rs
@@ -48,7 +48,6 @@ mod bindings {
     const BINDINGS_FILE: &'static str = "bindings.rs";
 
     fn read_config(path: &PathBuf) -> toml::Table {
-        println!("cargo:rerun-if-changed={}", path.to_str().unwrap());
         update_last_modified(&path);
 
         let mut contents = String::new();
@@ -143,7 +142,6 @@ mod bindings {
         let mut file = File::open(&path).unwrap();
         let mut content = String::new();
         file.read_to_string(&mut content).unwrap();
-        println!("cargo:rerun-if-changed={}", path.to_str().unwrap());
         added_paths.insert(path);
         // Find all includes and add them recursively
         for cap in INCLUDE_RE.captures_iter(&content) {
@@ -538,7 +536,6 @@ mod bindings {
     fn generate_atoms() {
         let script = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap())
             .join("gecko").join("regen_atoms.py");
-        println!("cargo:rerun-if-changed={}", script.display());
         let status = Command::new(&*PYTHON)
             .arg(&script)
             .arg(DISTDIR_PATH.as_os_str())
@@ -588,17 +585,14 @@ mod bindings {
 
     pub fn generate() {
         let dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()).join("gecko/generated");
-        println!("cargo:rerun-if-changed={}", dir.display());
         copy_dir(&dir, &*OUTDIR_PATH, |path| {
-            println!("cargo:rerun-if-changed={}", path.display());
         }).expect("Fail to copy generated files to out dir");
     }
 }
 
 pub fn generate() {
     use self::common::*;
     use std::fs;
-    println!("cargo:rerun-if-changed=build_gecko.rs");
     fs::create_dir_all(&*OUTDIR_PATH).unwrap();
     bindings::generate();
 }
diff --git a/collector/benchmarks/style-servo/components/style/gecko/regen_atoms.py b/collector/benchmarks/style-servo/components/style/gecko/regen_atoms.py
@@ -121,7 +121,6 @@ def collect_atoms(objdir):
     atoms = []
     for source in SOURCES:
         path = os.path.abspath(os.path.join(objdir, source.FILE))
-        print("cargo:rerun-if-changed={}".format(path))
         with open(path) as f:
             for line in f.readlines():
                 result = re.match(source.PATTERN, line)
@@ -242,7 +241,6 @@ def write_pseudo_elements(atoms, target_filename):
             pseudos.append(atom)
 
     pseudo_definition_template = os.path.join(GECKO_DIR, "pseudo_element_definition.mako.rs")
-    print("cargo:rerun-if-changed={}".format(pseudo_definition_template))
     contents = build.render(pseudo_definition_template, PSEUDOS=pseudos)
 
     with FileAvoidWrite(target_filename) as f:
diff --git a/collector/benchmarks/style-servo/rust-cssparser/build.rs b/collector/benchmarks/style-servo/rust-cssparser/build.rs
@@ -36,5 +36,5 @@ fn main() {
     let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap();
     let tokenizer_rs = Path::new(&manifest_dir).join("src/tokenizer.rs");
     codegen::main(&tokenizer_rs);
-    println!("cargo:rerun-if-changed={}", tokenizer_rs.display());
+    println!("cargo:rerun-if-changed=build.rs");
 }
diff --git a/collector/benchmarks/syn/perf-config.json b/collector/benchmarks/syn/perf-config.json
@@ -1,3 +1,4 @@
 {
-    "supports_stable": true
+    "supports_stable": true,
+    "touch_file": "src/lib.rs"
 }
diff --git a/collector/benchmarks/token-stream-stress/perf-config.json b/collector/benchmarks/token-stream-stress/perf-config.json
@@ -1,3 +1,4 @@
 {
-    "cargo_opts": "--bin token-stream-stress"
+    "cargo_opts": "--bin token-stream-stress",
+    "touch_file": "src/main.rs"
 }
diff --git a/collector/benchmarks/tokio-webpush-simple/perf-config.json b/collector/benchmarks/tokio-webpush-simple/perf-config.json
@@ -1,3 +1,4 @@
 {
-    "supports_stable": true
+    "supports_stable": true,
+    "touch_file": "src/main.rs"
 }
diff --git a/collector/benchmarks/webrender-wrench/0002-change-in-dependency.patch b/collector/benchmarks/webrender-wrench/0002-change-in-dependency.patch
diff --git a/collector/benchmarks/webrender/wrench/build.rs b/collector/benchmarks/webrender/wrench/build.rs
@@ -11,8 +11,8 @@ fn main() {
     let out_dir = env::var_os("OUT_DIR").unwrap();
     let out_dir = PathBuf::from(out_dir);
 
-    println!("cargo:rerun-if-changed=res/wrench.exe.manifest");
     if target.contains("windows") {
+        println!("cargo:rerun-if-changed=res/wrench.exe.manifest");
         let src = PathBuf::from("res/wrench.exe.manifest");
         let mut dst = out_dir
             .parent()
diff --git a/collector/benchmarks/wg-grammar/perf-config.json b/collector/benchmarks/wg-grammar/perf-config.json
@@ -0,0 +1,3 @@
+{
+    "touch_file": "src/lib.rs"
+}
diff --git a/collector/src/execute.rs b/collector/src/execute.rs
@@ -233,10 +233,11 @@ impl<'a> CargoProcess<'a> {
             // env vars set, but it doesn't hurt to have them.
             .env("RUSTC", &*FAKE_RUSTC)
             .env("RUSTC_REAL", &self.compiler.rustc)
-            .env(
-                "CARGO_INCREMENTAL",
-                &format!("{}", self.incremental as usize),
-            )
+            // We separately pass -Cincremental to the leaf crate --
+            // CARGO_INCREMENTAL is cached separately for both the leaf crate
+            // and any in-tree dependencies, and we don't want that; it wastes
+            // time.
+            .env("CARGO_INCREMENTAL", "0")
             .current_dir(cwd)
             .arg(subcommand)
             .arg("--manifest-path")
@@ -258,6 +259,13 @@ impl<'a> CargoProcess<'a> {
     }
 
     fn run_rustc(&mut self) -> anyhow::Result<()> {
+        log::info!(
+            "run_rustc with incremental={}, run_kind={:?}, patch={:?}",
+            self.incremental,
+            self.processor_etc.as_ref().map(|v| v.1),
+            self.processor_etc.as_ref().and_then(|v| v.3)
+        );
+
         loop {
             // Get the subcommand. If it's not `rustc` it must should be a
             // subcommand that itself invokes `rustc` (so that the `FAKE_RUSTC`
@@ -297,7 +305,7 @@ impl<'a> CargoProcess<'a> {
                 }
             }
             cmd.args(&self.cargo_args);
-            if log::log_enabled!(target: "raw_cargo_messages", log::Level::Trace) {
+            if env::var_os("CARGO_RECORD_TIMING").is_some() {
                 cmd.arg("-Zunstable-options");
                 cmd.arg("-Ztimings");
             }
@@ -310,6 +318,10 @@ impl<'a> CargoProcess<'a> {
             // we want to wrap rustc.
             if let Some((ref mut processor, ..)) = self.processor_etc {
                 let profiler = processor.profiler(self.build_kind).name();
+                // If we're using a processor, we expect that only the crate
+                // we're interested in benchmarking will be built, not any
+                // dependencies.
+                cmd.env("EXPECT_ONLY_WRAPPED_RUSTC", "1");
                 cmd.arg("--wrap-rustc-with");
                 cmd.arg(profiler);
                 cmd.args(&self.rustc_args);
@@ -335,6 +347,13 @@ impl<'a> CargoProcess<'a> {
                 }
             }
 
+            if self.incremental {
+                cmd.arg("-C");
+                let mut incr_arg = std::ffi::OsString::from("incremental=");
+                incr_arg.push(self.cwd.join("incremental-state"));
+                cmd.arg(incr_arg);
+            }
+
             log::debug!("{:?}", cmd);
 
             let output = command_output(&mut cmd)?;
diff --git a/collector/src/rustc-fake.rs b/collector/src/rustc-fake.rs
@@ -215,6 +215,12 @@ fn main() {
             }
         }
     } else {
+        if env::var_os("EXPECT_ONLY_WRAPPED_RUSTC").is_some() {
+            eprintln!("{:?} {:?}", tool, args);
+            eprintln!("exiting -- non-wrapped rustc");
+            std::process::exit(1);
+        }
+
         let mut cmd = Command::new(&tool);
         cmd.args(&args);
         exec(&mut cmd);

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`{`
`2`	`2`	`"cargo_opts": "--lib",`
`3`		`- "runs": 1`
	`3`	`+ "runs": 1,`
	`4`	`+ "touch_file": "src/cargo/lib.rs"`
`4`	`5`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
`1`	`1`	`{`
`2`		`- "supports_stable": true`
	`2`	`+ "supports_stable": true,`
	`3`	`+ "touch_file": "src/lib.rs"`
`3`	`4`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+{`
	`2`	`+ "touch_file": "src/lib.rs"`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+{`
	`2`	`+ "touch_file": "src/main.rs"`
	`3`	`+}`