diff --git a/.github/.cspell/project-dictionary.txt b/.github/.cspell/project-dictionary.txt index b248788a..aeeb2fff 100644 --- a/.github/.cspell/project-dictionary.txt +++ b/.github/.cspell/project-dictionary.txt @@ -6,6 +6,7 @@ doctestbins easytime fcoverage fprofile +instrprof libdir libhello microkernel diff --git a/CHANGELOG.md b/CHANGELOG.md index 14a8694b..c8ff9e7f 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/README.md b/README.md index 11106f7c..a1d69469 100644 --- a/README.md +++ b/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 diff --git a/src/cli.rs b/src/cli.rs index cd6febb5..5bd28418 100644 --- a/src/cli.rs +++ b/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), diff --git a/src/main.rs b/src/main.rs index 181b5e98..b79cc9e0 100644 --- a/src/main.rs +++ b/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,7 +299,7 @@ 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)?; @@ -283,12 +307,6 @@ fn set_env(cx: &Context, env: &mut dyn EnvTarget, IsNextest(is_nextest): IsNexte } 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 ' cargo::test_or_run_args(cx, &mut cargo); if term::verbose() { status!("Running", "{cargo}");