cargo llvm-cov nextest no multithreading for funtional tests?

[Dushistov]

In crate I have "unit" tests and "functional" one.

// src/lib.rs
pub fn sleep(secs: u32) {
    std::thread::sleep(std::time::Duration::from_secs(secs.into()));
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_8() {
        sleep(8);
    }
}

// tests/functional_tests.rs
use crate_example_with_tests::*;

#[test]
fn test_30() {
    sleep(30);
}

Because of number of CPU on my machine is >=2 I expect that execution time cargo nextest
is equal to time of the slowest test - test_30, and it works as expected:

❯ cargo nextest run
   Compiling crate_example_with_tests v0.1.0 (/home/evgeniy/bigdisk1/projects/rust-infra/crate_example_with_tests)
    Finished test [unoptimized + debuginfo] target(s) in 0.26s
    Starting 2 tests across 2 binaries
        PASS [   8.003s] crate_example_with_tests tests::test_8
        PASS [  30.003s] crate_example_with_tests::functional_tests test_30
------------
     Summary [  30.003s] 2 tests run: 2 passed, 0 skipped

but with llvm-cov it for some reasons waits all unit tests to pass, before starting functional tests:

❯ cargo llvm-cov nextest
   Compiling crate_example_with_tests v0.1.0 (/home/evgeniy/bigdisk1/projects/rust-infra/crate_example_with_tests)
    Finished test [unoptimized + debuginfo] target(s) in 0.33s
    Starting 2 tests across 2 binaries
        PASS [   8.003s] crate_example_with_tests tests::test_8
        PASS [  30.004s] crate_example_with_tests::functional_tests test_30
------------
     Summary [  38.008s] 2 tests run: 2 passed, 0 skipped

On real project all looks worse.
Is any way to run functional and unit tests in parallel with llvm-cov?

[taiki-e]

cargo-llvm-cov limits the parallelism in test binaries to avoid the rustc bug. IIRC cargo-nextest does not respect RUST_TEST_THREADS (nextest-rs/nextest#652), so in cargo llvm-cov nextest this is controlled using NEXTEST_TEST_THREADS environment variable. However, NEXTEST_TEST_THREADS seems to also limit overall parallelism.

@sunshowers: Is there a way to control only the number of threads created by test binaries without affecting the overall parallelism? (In other words, is there an equivalent option to RUST_TEST_THREADS?)

[sunshowers]

@taiki-e I think RUST_TEST_THREADS should work for that -- nextest doesn't do anything with it, and I think RUST_TEST_THREADS is read by libtest.

[taiki-e]

Hmm, at the time of nextest-rs/nextest#652 reported, we already set RUST_TEST_THREADS=1, but it did not seem to be enough to avoid the rustc bug.

[sunshowers]

In that case I think something in rustc or llvm-cov might not support multiple test binaries being run in parallel.

[Dushistov]

So, may be just pass -C llvm-args=--instrprof-atomic-counter-update-all,
and remove setting of RUST_TEST_THREADS to 1?

[taiki-e]

Yes, at least RUST_TEST_THREADS=1 can be replaced by -C llvm-args=--instrprof-atomic-counter-update-all. It is not clear yet if NEXTEST_TEST_THREADS can be removed.

[taiki-e]

If we can understand how cargo-nextest determines the number of tests to run simultaneously, we may also avoid the need to set NEXTEST_TEST_THREADS by using %Nm.

https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#running-the-instrumented-program

“%Nm” expands out to the instrumented binary’s signature. When this pattern is specified, the runtime creates a pool of N raw profiles which are used for on-line profile merging. The runtime takes care of selecting a raw profile from the pool, locking it, and updating it before the program exits. If N is not specified (i.e the pattern is “%m”), it’s assumed that N = 1. The merge pool specifier can only occur once per filename pattern.

For a program such as the Lit testing tool which invokes other programs, it may be necessary to set LLVM_PROFILE_FILE for each invocation. The pattern strings “%p” or “%Nm” may help to avoid corruption due to concurrency. Note that “%p” is also a Lit token and needs to be escaped as “%%p”.

we currnetly sets %m (i.e., %1m)

cargo-llvm-cov/src/main.rs

Line 201 in 752a20e

let llvm_profile_file = cx.ws.target_dir.join(format!("{}-%p-%m.profraw", cx.ws.name));

[sunshowers]

Would it make sense to add a command to nextest showing the effective config along with its source?

[taiki-e]

Hmm, it may not actually be necessary to know the exact value specified in config. Here is a todo comment I added in #279:

cargo-llvm-cov/src/main.rs

Lines 208 to 217 in ee3e3b8

	// 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.