Skip to content

Commit 311bd43

Browse files
committed
WIP: Options for sharding strategies
1 parent 8f396d4 commit 311bd43

File tree

3 files changed

+81
-19
lines changed

3 files changed

+81
-19
lines changed

src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ fn main() -> Result<()> {
560560
};
561561
}
562562
if let Some(shard) = &args.shard {
563-
mutants = shard.select(mutants);
563+
mutants = options.sharding().shard(*shard, mutants);
564564
}
565565
if args.list {
566566
print!("{}", list_mutants(&mutants, &options));

src/options.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use crate::annotation::ResolvedAnnotation;
2828
use crate::config::Config;
2929
use crate::glob::build_glob_set;
3030
use crate::mutant::Mutant;
31+
use crate::shard::Sharding;
3132
use crate::{Args, BaselineStrategy, Phase, Result, ValueEnum};
3233

3334
/// Options for mutation testing, based on both command-line arguments and the config file.
@@ -180,6 +181,10 @@ pub struct Common {
180181
/// Tool used to run test suites: cargo or nextest.
181182
#[arg(long, help_heading = "Execution")]
182183
pub test_tool: Option<TestTool>,
184+
185+
/// Sharding method to use when running with --shards.
186+
#[arg(long, help_heading = "Execution", requires = "shard")]
187+
pub sharding: Option<Sharding>,
183188
}
184189

185190
impl Common {
@@ -190,6 +195,7 @@ impl Common {
190195
Common {
191196
emit_diffs: self.emit_diffs.or(other.emit_diffs),
192197
test_tool: self.test_tool.or(other.test_tool),
198+
sharding: self.sharding.or(other.sharding),
193199
}
194200
}
195201
}
@@ -473,6 +479,10 @@ impl Options {
473479
pub fn test_tool(&self) -> TestTool {
474480
self.common.test_tool.unwrap_or_default()
475481
}
482+
483+
pub fn sharding(&self) -> Sharding {
484+
self.common.sharding.unwrap_or_default()
485+
}
476486
}
477487

478488
/// If the first slices is non-empty, return that, otherwise the second.

src/shard.rs

Lines changed: 70 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
// Copyright 2023 Martin Pool
2-
31
//! Sharding parameters.
42
53
use std::str::FromStr;
64

75
use anyhow::{anyhow, ensure, Context, Error};
6+
use clap::ValueEnum;
7+
use schemars::JsonSchema;
8+
use serde::Deserialize;
89

910
/// Select mutants for a particular shard of the total list.
1011
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
@@ -15,17 +16,6 @@ pub struct Shard {
1516
pub n: usize,
1617
}
1718

18-
impl Shard {
19-
/// Select the mutants that should be run for this shard.
20-
pub fn select<M, I: IntoIterator<Item = M>>(&self, mutants: I) -> Vec<M> {
21-
mutants
22-
.into_iter()
23-
.enumerate()
24-
.filter_map(|(i, m)| if i % self.n == self.k { Some(m) } else { None })
25-
.collect()
26-
}
27-
}
28-
2919
impl FromStr for Shard {
3020
type Err = Error;
3121

@@ -38,6 +28,57 @@ impl FromStr for Shard {
3828
}
3929
}
4030

31+
/// Method for sharding.
32+
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Deserialize, ValueEnum, JsonSchema)]
33+
#[serde(rename_all = "snake_case")]
34+
pub enum Sharding {
35+
/// Run mutant `i` on shard `i % k`.
36+
///
37+
/// This was the default up to cargo-mutants 25.3.1.
38+
///
39+
/// This distributes mutants more evenly and will likely generate more equal completion
40+
/// times, but it has less locality of reference within each shard's cache and so may
41+
/// cause more build time.
42+
#[default]
43+
RoundRobin,
44+
45+
/// Run consecutive ranges of mutants on each shard: `0..(n/k)` on shard 0, etc. (default)
46+
///
47+
/// This makes the incremental change between each build likely to be smaller and
48+
/// so may reduce build time, but it may also lead to more unbalanced shards.
49+
Slice,
50+
}
51+
52+
impl Sharding {
53+
/// Select the mutants that should be run for this shard.
54+
pub fn shard<M>(self, shard: Shard, mut mutants: Vec<M>) -> Vec<M> {
55+
match self {
56+
Sharding::RoundRobin => mutants
57+
.into_iter()
58+
.enumerate()
59+
.filter_map(|(i, m)| {
60+
if i % shard.n == shard.k {
61+
Some(m)
62+
} else {
63+
None
64+
}
65+
})
66+
.collect(),
67+
Sharding::Slice => {
68+
let total = mutants.len();
69+
let chunk_size = total.div_ceil(shard.n);
70+
let start = shard.k * chunk_size;
71+
let end = ((shard.k + 1) * chunk_size).min(total);
72+
if start >= total {
73+
Vec::new()
74+
} else {
75+
mutants.drain(start..end).collect()
76+
}
77+
}
78+
}
79+
}
80+
}
81+
4182
#[cfg(test)]
4283
mod tests {
4384
use super::*;
@@ -74,10 +115,21 @@ mod tests {
74115
}
75116

76117
#[test]
77-
fn shard_select() {
78-
assert_eq!(
79-
Shard::from_str("1/4").unwrap().select(0..10).as_slice(),
80-
&[1, 5, 9]
81-
);
118+
fn shard_round_robin() {
119+
// This test works on ints instead of real mutants just for ease of testing.
120+
let fake_mutants: Vec<usize> = (0..10).collect();
121+
for (k, expect) in [
122+
(0, [0, 4, 8].as_slice()),
123+
(1, &[1, 5, 9]),
124+
(2, &[2, 6]),
125+
(3, &[3, 7]),
126+
] {
127+
assert_eq!(
128+
Sharding::RoundRobin
129+
.shard(Shard { k, n: 4 }, fake_mutants.clone())
130+
.as_slice(),
131+
expect
132+
);
133+
}
82134
}
83135
}

0 commit comments

Comments
 (0)