Skip to content

Commit

Permalink
Increase timeouts proportional to the number of jobs (#367)
Browse files Browse the repository at this point in the history 
Fixes #365
  • Loading branch information
@sourcefrog
sourcefrog authored Jul 5, 2024
2 parents e2c6463 + 6feefe9 commit 153d41f
Show file tree
Hide file tree
Showing 5 changed files with 292 additions and 247 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# cargo-mutants changelog

## Unreleased

- Fixed: The auto-set timeout for building mutants is now 2 times the baseline build time times the number of jobs, with a minimum of 20 seconds. This was changed because builds of mutants contend with each other for access to CPUs and may be slower than the baseline build.

## 24.5.0

- Fixed: Follow `path` attributes on `mod` statements.
Expand Down
4 changes: 3 additions & 1 deletion book/src/timeouts.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ build and test the unmodified tree (baseline).

The default timeouts are:

- `cargo build`/`cargo check`: 2 times the baseline build time (with a minimum of 20 seconds)
- `cargo build`/`cargo check`: 2 times the baseline build time times the number of jobs (with a minimum of 20 seconds)
- `cargo test`: 5 times baseline test time (with a minimum of 20 seconds)

The build timeout scales with the number of jobs to reflect that cargo often spawns many jobs, and so builds run in parallel are likely to take longer than the baseline, which has no external parallelism.

The minimum of 20 seconds for the test timeout can be overridden by the
`--minimum-test-timeout` option or the `CARGO_MUTANTS_MINIMUM_TEST_TIMEOUT`
environment variable, measured in seconds.
Expand Down
256 changes: 10 additions & 246 deletions src/lab.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::cmp::{max, min};
use std::panic::resume_unwind;
use std::sync::Mutex;
use std::thread;
use std::time::{Duration, Instant};
use std::time::Instant;

use itertools::Itertools;
use tracing::warn;
Expand All @@ -17,6 +17,7 @@ use crate::cargo::run_cargo;
use crate::outcome::LabOutcome;
use crate::output::OutputDir;
use crate::package::Package;
use crate::timeouts::Timeouts;
use crate::*;

/// Run all possible mutation experiments.
Expand Down Expand Up @@ -53,22 +54,18 @@ pub fn test_mutants(
debug!(?all_packages);

let output_mutex = Mutex::new(output_dir);
let build_dir = if options.in_place {
BuildDir::in_place(workspace_dir)?
} else {
BuildDir::copy_from(workspace_dir, options.gitignore, options.leak_dirs, console)?
let build_dir = match options.in_place {
true => BuildDir::in_place(workspace_dir)?,
false => BuildDir::copy_from(workspace_dir, options.gitignore, options.leak_dirs, console)?,
};
let baseline_outcome = match options.baseline {
let timeouts = match options.baseline {
BaselineStrategy::Run => {
let outcome = test_scenario(
&build_dir,
&output_mutex,
&Scenario::Baseline,
&all_packages,
Timeouts {
test: options.test_timeout.unwrap_or(Duration::MAX),
build: options.build_timeout.unwrap_or(Duration::MAX),
},
Timeouts::for_baseline(&options),
&options,
console,
)?;
Expand All @@ -84,23 +81,12 @@ pub fn test_mutants(
.expect("lock output_dir")
.take_lab_outcome());
}
Some(outcome)
Timeouts::from_baseline(&outcome, &options)
}
BaselineStrategy::Skip => None,
BaselineStrategy::Skip => Timeouts::without_baseline(&options),
};
debug!(?timeouts);
let mut build_dirs = vec![build_dir];

let baseline_duration_by_phase = |phase| {
baseline_outcome
.as_ref()
.and_then(|so| so.phase_result(phase))
.map(|pr| pr.duration)
};
let timeouts = Timeouts {
build: build_timeout(baseline_duration_by_phase(Phase::Build), &options),
test: test_timeout(baseline_duration_by_phase(Phase::Test), &options),
};

let jobs = max(1, min(options.jobs.unwrap_or(1), mutants.len()));
for i in 1..jobs {
debug!("copy build dir {i}");
Expand Down Expand Up @@ -198,78 +184,6 @@ pub fn test_mutants(
Ok(lab_outcome)
}

#[derive(Copy, Clone)]
struct Timeouts {
build: Duration,
test: Duration,
}

fn phase_timeout(
phase: Phase,
explicit_timeout: Option<Duration>,
baseline_duration: Option<Duration>,
minimum: Duration,
multiplier: f64,
options: &Options,
) -> Duration {
const FALLBACK_TIMEOUT_SECS: u64 = 300;
fn warn_fallback_timeout(phase_name: &str, option: &str) {
warn!("An explicit {phase_name} timeout is recommended when using {option}; using {FALLBACK_TIMEOUT_SECS} seconds by default");
}

if let Some(timeout) = explicit_timeout {
return timeout;
}

match baseline_duration {
Some(_) if options.in_place && phase != Phase::Test => {
warn_fallback_timeout(phase.name(), "--in-place");
Duration::from_secs(FALLBACK_TIMEOUT_SECS)
}
Some(baseline_duration) => {
let timeout = max(
minimum,
Duration::from_secs((baseline_duration.as_secs_f64() * multiplier).ceil() as u64),
);

if options.show_times {
info!(
"Auto-set {} timeout to {}",
phase.name(),
humantime::format_duration(timeout)
);
}
timeout
}
None => {
warn_fallback_timeout(phase.name(), "--baseline=skip");
Duration::from_secs(FALLBACK_TIMEOUT_SECS)
}
}
}

fn test_timeout(baseline_duration: Option<Duration>, options: &Options) -> Duration {
phase_timeout(
Phase::Test,
options.test_timeout,
baseline_duration,
options.minimum_test_timeout,
options.test_timeout_multiplier.unwrap_or(5.0),
options,
)
}

fn build_timeout(baseline_duration: Option<Duration>, options: &Options) -> Duration {
phase_timeout(
Phase::Build,
options.build_timeout,
baseline_duration,
Duration::from_secs(20),
options.build_timeout_multiplier.unwrap_or(2.0),
options,
)
}

/// Test various phases of one scenario in a build dir.
///
/// The [BuildDir] is passed as mutable because it's for the exclusive use of this function for the
Expand Down Expand Up @@ -337,153 +251,3 @@ fn test_scenario(

Ok(outcome)
}

#[cfg(test)]
mod test {
use std::str::FromStr;

use indoc::indoc;

use super::*;
use crate::config::Config;

#[test]
fn timeout_multiplier_from_option() {
let args = Args::parse_from(["mutants", "--timeout-multiplier", "1.5"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(options.test_timeout_multiplier, Some(1.5));
assert_eq!(
test_timeout(Some(Duration::from_secs(40)), &options),
Duration::from_secs(60),
);
}

#[test]
fn test_timeout_unaffected_by_in_place_build() {
let args = Args::parse_from(["mutants", "--timeout-multiplier", "1.5", "--in-place"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(
test_timeout(Some(Duration::from_secs(40)), &options),
Duration::from_secs(60),
);
}

#[test]
fn build_timeout_multiplier_from_option() {
let args = Args::parse_from(["mutants", "--build-timeout-multiplier", "1.5"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(options.build_timeout_multiplier, Some(1.5));
assert_eq!(
build_timeout(Some(Duration::from_secs(40)), &options),
Duration::from_secs(60),
);
}

#[test]
fn build_timeout_is_affected_by_in_place_build() {
let args = Args::parse_from(["mutants", "--build-timeout-multiplier", "1.5", "--in-place"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(
build_timeout(Some(Duration::from_secs(40)), &options),
Duration::from_secs(300),
);
}

#[test]
fn timeout_multiplier_from_config() {
let args = Args::parse_from(["mutants"]);
let config = Config::from_str(indoc! {r#"
timeout_multiplier = 2.0
"#})
.unwrap();
let options = Options::new(&args, &config).unwrap();

assert_eq!(options.test_timeout_multiplier, Some(2.0));
assert_eq!(
test_timeout(Some(Duration::from_secs(42)), &options),
Duration::from_secs(42 * 2),
);
}

#[test]
fn build_timeout_multiplier_from_config() {
let args = Args::parse_from(["mutants"]);
let config = Config::from_str(indoc! {r#"
build_timeout_multiplier = 2.0
"#})
.unwrap();
let options = Options::new(&args, &config).unwrap();

assert_eq!(options.build_timeout_multiplier, Some(2.0));
assert_eq!(
build_timeout(Some(Duration::from_secs(42)), &options),
Duration::from_secs(42 * 2),
);
}

#[test]
fn timeout_multiplier_default() {
let args = Args::parse_from(["mutants"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(options.test_timeout_multiplier, None);
assert_eq!(
test_timeout(Some(Duration::from_secs(42)), &options),
Duration::from_secs(42 * 5),
);
}

#[test]
fn build_timeout_multiplier_default() {
let args = Args::parse_from(["mutants"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(options.build_timeout_multiplier, None);
assert_eq!(
build_timeout(Some(Duration::from_secs(42)), &options),
Duration::from_secs(42 * 2),
);
}

#[test]
fn timeout_from_option() {
let args = Args::parse_from(["mutants", "--timeout=8"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(options.test_timeout, Some(Duration::from_secs(8)));
}

#[test]
fn build_timeout_from_option() {
let args = Args::parse_from(["mutants", "--build-timeout=4"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(options.build_timeout, Some(Duration::from_secs(4)));
}

#[test]
fn timeout_multiplier_default_with_baseline_skip() {
// The --baseline option is not used to set the timeout but it's
// indicative of the realistic situation.
let args = Args::parse_from(["mutants", "--baseline", "skip"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(options.test_timeout_multiplier, None);
assert_eq!(test_timeout(None, &options), Duration::from_secs(300),);
}

#[test]
fn build_timeout_multiplier_default_with_baseline_skip() {
// The --baseline option is not used to set the timeout but it's
// indicative of the realistic situation.
let args = Args::parse_from(["mutants", "--baseline", "skip"]);
let options = Options::new(&args, &Config::default()).unwrap();

assert_eq!(options.build_timeout_multiplier, None);
assert_eq!(build_timeout(None, &options), Duration::from_secs(300),);
}
}
1 change: 1 addition & 0 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ mod shard;
mod source;
mod span;
mod tail_file;
mod timeouts;
mod visit;
mod workspace;

Expand Down
Loading

0 comments on commit 153d41f

Please sign in to comment.