Use llvm-args instead of RUST_TEST_THREADS, %Nm instead of NEXTEST_TEST_THREADS

.github/.cspell/project-dictionary.txt

@@ -6,6 +6,7 @@ doctestbins
 easytime
 fcoverage
 fprofile
+instrprof
 libdir
 libhello
 microkernel

CHANGELOG.md

@@ -10,6 +10,10 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 ## [Unreleased]
 
+- cargo-llvm-cov no longer sets the `RUST_TEST_THREADS` and `NEXTEST_TEST_THREADS` environment variables. cargo-llvm-cov now adopts another efficient way to workaround [rust-lang/rust#91092](https://github.com/rust-lang/rust/issues/91092). ([#279](https://github.com/taiki-e/cargo-llvm-cov/pull/279))
+
+  This may greatly improve performance, [especially when using `cargo llvm-cov nextest`](https://github.com/taiki-e/cargo-llvm-cov/pull/279#issuecomment-1552058044).
+
 ## [0.5.19] - 2023-04-28
 
 - Fix handling of `--cargo-profile` option for `cargo llvm-cov nextest`. ([#269](https://github.com/taiki-e/cargo-llvm-cov/pull/269))

README.md

@@ -622,7 +622,6 @@ pacman -S cargo-llvm-cov
 
 - Branch coverage is not supported yet. See [#8] and [rust-lang/rust#79649] for more.
 - Support for doc tests is unstable and has known issues. See [#2] and [rust-lang/rust#79417] for more.
-- All the tests are run with `RUST_TEST_THREADS=1` to work around [rust-lang/rust#91092]. You can pass `--test-threads` (e.g., `--test-threads=$(nproc)`) to override this behavior. (As for `nextest`, we also set `NEXTEST_TEST_THREADS=1`.)
 
 See also [the code-coverage-related issues reported in rust-lang/rust](https://github.com/rust-lang/rust/labels/A-code-coverage).
 
@@ -648,7 +647,6 @@ See also [the code-coverage-related issues reported in rust-lang/rust](https://g
 [rust-lang/rust#79417]: https://github.com/rust-lang/rust/issues/79417
 [rust-lang/rust#79649]: https://github.com/rust-lang/rust/issues/79649
 [rust-lang/rust#84605]: https://github.com/rust-lang/rust/issues/84605
-[rust-lang/rust#91092]: https://github.com/rust-lang/rust/issues/91092
 [xtask]: https://github.com/matklad/cargo-xtask
 
 ## License

src/cli.rs

@@ -358,10 +358,10 @@ impl Args {
                 // build options
                 Short('r') | Long("release") => parse_flag_passthrough!(release),
                 Long("profile") if subcommand != Subcommand::Nextest => {
-                    parse_opt_passthrough!(profile)
+                    parse_opt_passthrough!(profile);
                 }
                 Long("cargo-profile") if subcommand == Subcommand::Nextest => {
-                    parse_opt_passthrough!(profile)
+                    parse_opt_passthrough!(profile);
                 }
                 Long("target") => parse_opt_passthrough!(target),
                 Long("coverage-target-only") => parse_flag!(coverage_target_only),

src/main.rs

@@ -190,6 +190,10 @@ fn set_env(cx: &Context, env: &mut dyn EnvTarget, IsNextest(is_nextest): IsNexte
                 flags.push("codegen-units=1");
             }
         }
+        // Workaround for https://github.com/rust-lang/rust/issues/91092
+        // TODO: skip passing this on newer nightly that includes https://github.com/rust-lang/rust/pull/111469.
+        flags.push("-C");
+        flags.push("llvm-args=--instrprof-atomic-counter-update-all");
         if !cx.args.cov.no_cfg_coverage {
             flags.push("--cfg=coverage");
         }
@@ -198,7 +202,27 @@ fn set_env(cx: &Context, env: &mut dyn EnvTarget, IsNextest(is_nextest): IsNexte
         }
     }
 
-    let llvm_profile_file = cx.ws.target_dir.join(format!("{}-%p-%m.profraw", cx.ws.name));
+    let llvm_profile_file = if is_nextest {
+        // https://github.com/taiki-e/cargo-llvm-cov/issues/258
+        // https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#running-the-instrumented-program
+        // Select the number of threads that is the same as the one nextest uses by default here.
+        // https://github.com/nextest-rs/nextest/blob/c54694dfe7be016993983b5dedbcf2b50d4b1a6e/nextest-runner/src/config/test_threads.rs
+        // https://github.com/nextest-rs/nextest/blob/c54694dfe7be016993983b5dedbcf2b50d4b1a6e/nextest-runner/src/config/config_impl.rs#L30
+        // TODO: should we respect custom test-threads?
+        // - If the number of threads specified by the user is negative or
+        //   less or equal to available cores, it should not really be a problem
+        //   because it does not exceed the number of available cores.
+        // - Even if the number of threads specified by the user is greater than
+        //   available cores, it is expected that the number of threads that can
+        //   write simultaneously will not exceed the number of available cores.
+        cx.ws.target_dir.join(format!(
+            "{}-%p-%{}m.profraw",
+            cx.ws.name,
+            std::thread::available_parallelism().map_or(1, usize::from)
+        ))
+    } else {
+        cx.ws.target_dir.join(format!("{}-%p-%m.profraw", cx.ws.name))
+    };
 
     let rustflags = &mut cx.ws.config.rustflags(&cx.ws.target_for_config)?.unwrap_or_default();
     push_common_flags(cx, rustflags);
@@ -275,20 +299,14 @@ fn set_env(cx: &Context, env: &mut dyn EnvTarget, IsNextest(is_nextest): IsNexte
                 Err(_) => std::env::var("CXXFLAGS").unwrap_or_default(),
             },
         };
-        let clang_flags = " -fprofile-instr-generate -fcoverage-mapping";
+        let clang_flags = " -fprofile-instr-generate -fcoverage-mapping -fprofile-update=atomic";
         cflags.push_str(clang_flags);
         cxxflags.push_str(clang_flags);
         env.set(cflags_key, &cflags)?;
         env.set(cxxflags_key, &cxxflags)?;
     }
     env.set("LLVM_PROFILE_FILE", llvm_profile_file.as_str())?;
     env.set("CARGO_INCREMENTAL", "0")?;
-    // Workaround for https://github.com/rust-lang/rust/issues/91092
-    env.set("RUST_TEST_THREADS", "1")?;
-    if is_nextest {
-        // Same as above
-        env.set("NEXTEST_TEST_THREADS", "1")?;
-    }
     env.set("CARGO_LLVM_COV", "1")?;
     Ok(())
 }
@@ -370,7 +388,6 @@ fn run_nextest(cx: &Context) -> Result<()> {
         {
             let mut cargo = cargo.clone();
             cargo.arg("--no-run");
-            cargo.env_remove("NEXTEST_TEST_THREADS"); // error: the argument '--no-run' cannot be used with '--test-threads <THREADS>'
             cargo::test_or_run_args(cx, &mut cargo);
             if term::verbose() {
                 status!("Running", "{cargo}");