Skip to content

Commit 5d31d14

Browse files
committed
Make --include and --exclude use exact matching.
Currently they do substring matching. But because some benchmarks have a name that is a substring of another it's impossible to match some benchmarks by themselves. E.g.: - Want `webrender`? You'll get `webrender-wrench` too. - Want `syn`? You'll get `deeply-nested-async` too. - Want `deeply-nested`? You'll get `deeply-nested-async` and `deeply-nested-closures` too. This commit makes the matching exact. As a heavy user of these tools I think this is a good trade-off. It makes some things possible that are currently impossible (e.g. including or excluding just `webrender`), at the cost of more typing on occasion, e.g. `--include ctfe-stress-4` instead of `--include ctfe`. (Note: if you specify something that doesn't match any benchmark's name the command will give an error.)
1 parent 307bf1a commit 5d31d14

File tree

2 files changed

+36
-26
lines changed

2 files changed

+36
-26
lines changed

collector/README.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,13 @@ The following options alter the behaviour of the `bench_local` subcommand.
100100
supports postgres as a backend and the URL can be specified (beginning with
101101
`postgres://`), but this is unlikely to be useful for local collection.
102102
- `--exclude <EXCLUDE>`: this is used to run a subset of the benchmarks. The
103-
argument is a comma-separated list of strings. When this option is specified,
104-
a benchmark is excluded from the run if its name contains one or more of the
105-
given strings.
103+
argument is a comma-separated list of benchmark names. When this option is
104+
specified, a benchmark is excluded from the run if its name matches one or
105+
more of the given names.
106106
- `--include <INCLUDE>`: the inverse of `--exclude`. The argument is a
107-
comma-separated list of strings. When this option is specified, a benchmark
108-
is included in the run only if its name contains one or more of the given
109-
strings.
107+
comma-separated list of benchmark names. When this option is specified, a
108+
benchmark is included in the run only if its name matches one or more of the
109+
given names.
110110
- `--runs $RUNS`: the run kinds to be benchmarked. The possible choices are one
111111
or more (comma-separated) of `Full`, `IncrFull`, `IncrUnchanged`,
112112
`IncrPatched`, and `All`. The default is `All`. Note that `IncrFull` is

collector/src/main.rs

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -375,32 +375,42 @@ fn get_benchmarks(
375375
}
376376
paths.push((PathBuf::from("rustc"), String::from("rustc")));
377377

378-
'outer: for (path, name) in paths {
379-
if let Some(include) = include {
380-
if !include
381-
.split(',')
382-
.any(|to_include| name.contains(to_include))
383-
{
378+
let mut includes = include.map(|list| list.split(',').collect::<HashSet<_>>());
379+
let mut excludes = exclude.map(|list| list.split(',').collect::<HashSet<_>>());
380+
381+
for (path, name) in paths {
382+
if let Some(includes) = includes.as_mut() {
383+
if !includes.remove(name.as_str()) {
384384
debug!(
385-
"benchmark {} - doesn't match --include argument, skipping",
385+
"benchmark {} - not named by --include argument, skipping",
386386
name
387387
);
388-
continue 'outer;
388+
continue;
389389
}
390390
}
391391

392-
if let Some(exclude) = exclude {
393-
for exc in exclude.split(',') {
394-
if name.contains(exc) {
395-
debug!("benchmark {} - matches --exclude argument, skipping", name);
396-
continue 'outer;
397-
}
392+
if let Some(excludes) = excludes.as_mut() {
393+
if excludes.remove(name.as_str()) {
394+
debug!("benchmark {} - named by --exclude argument, skipping", name);
395+
continue;
398396
}
399397
}
400398

401399
debug!("benchmark `{}`- registered", name);
402400
benchmarks.push(Benchmark::new(name, path)?);
403401
}
402+
403+
if let Some(includes) = includes {
404+
if !includes.is_empty() {
405+
bail!("Warning: one or more invalid --include entries: {:?}", includes);
406+
}
407+
}
408+
if let Some(excludes) = excludes {
409+
if !excludes.is_empty() {
410+
bail!("Warning: one or more invalid --exclude entries: {:?}", excludes);
411+
}
412+
}
413+
404414
benchmarks.sort_by_key(|benchmark| benchmark.name.clone());
405415

406416
if benchmarks.is_empty() {
@@ -717,10 +727,10 @@ fn main_result() -> anyhow::Result<i32> {
717727
(@arg CARGO: --cargo +takes_value "The path to the local Cargo to use")
718728
(@arg DB: --db +takes_value "Database output file")
719729
(@arg EXCLUDE: --exclude +takes_value
720-
"Exclude all benchmarks matching anything in\n\
730+
"Exclude all benchmarks that are listed in\n\
721731
this comma-separated list of patterns")
722732
(@arg INCLUDE: --include +takes_value
723-
"Include only benchmarks matching something in\n\
733+
"Include only benchmarks that are listed in\n\
724734
this comma-separated list of patterns")
725735
(@arg RUNS: --runs +takes_value
726736
"One or more (comma-separated) of: 'Full',\n\
@@ -769,10 +779,10 @@ fn main_result() -> anyhow::Result<i32> {
769779
'Debug', 'Doc', 'Opt', 'All'")
770780
(@arg CARGO: --cargo +takes_value "The path to the local Cargo to use")
771781
(@arg EXCLUDE: --exclude +takes_value
772-
"Exclude all benchmarks matching anything in\n\
782+
"Exclude all benchmarks that are listed in\n\
773783
this comma-separated list of patterns")
774784
(@arg INCLUDE: --include +takes_value
775-
"Include only benchmarks matching something in\n\
785+
"Include only benchmarks that are listed in\n\
776786
this comma-separated list of patterns")
777787
(@arg OUT_DIR: --("out-dir") +takes_value "Output directory")
778788
(@arg RUNS: --runs +takes_value
@@ -798,10 +808,10 @@ fn main_result() -> anyhow::Result<i32> {
798808
'Debug', 'Doc', 'Opt', 'All'")
799809
(@arg CARGO: --cargo +takes_value "The path to the local Cargo to use")
800810
(@arg EXCLUDE: --exclude +takes_value
801-
"Exclude all benchmarks matching anything in\n\
811+
"Exclude all benchmarks that are listed in\n\
802812
this comma-separated list of patterns")
803813
(@arg INCLUDE: --include +takes_value
804-
"Include only benchmarks matching something in\n\
814+
"Include only benchmarks that are listed in\n\
805815
this comma-separated list of patterns")
806816
(@arg OUT_DIR: --("out-dir") +takes_value "Output directory")
807817
(@arg RUNS: --runs +takes_value

0 commit comments

Comments
 (0)