diff options
author | Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> | 2024-03-26 09:19:24 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-03-26 09:19:24 -0700 |
commit | a2a537e196360c5e44821f3bff901bd478ab8156 (patch) | |
tree | 39bb427d5abb7b75d2ed9eba9c8655728985f99c | |
parent | 89aa6d5cf6ae76e3698587923453f535b9af81af (diff) |
fix(bench): Fix group header printing logic + don't filter out the warmup benchmark (#23083)
Fixes #23053.
Two small bugs here:
- the existing condition for printing out the group header was broken.
it worked in the reproducer (in the issue above) without filtering only
by accident, due to setting `self.has_ungrouped = true` once we see the
warmup bench. Knowing that we sort benchmarks to put ungrouped benches
first, there are only two cases: 1) we are starting the first group 2)
we are ending the previous group and starting a new group
- when you passed `--filter` we were applying that filter to the warmup
bench (which is not visible to users), so we suffered from jit bias if
you were filtering (unless your filter was `<warmup>`)
TLDR;
Running
```bash
deno bench main.js --filter="G"
```
```js
// main.js
Deno.bench({
group: "G1",
name: "G1-A",
fn() {},
});
Deno.bench({
group: "G1",
name: "G1-B",
fn() {},
});
```
Before this PR:
```
benchmark time (avg) iter/s (min … max) p75 p99 p995
--------------------------------------------------------------- -----------------------------
G1-A 303.52 ps/iter3,294,726,102.1 (254.2 ps … 7.8 ns) 287.5 ps 391.7 ps 437.5 ps
G1-B 3.8 ns/iter 263,360,635.9 (2.24 ns … 8.36 ns) 3.84 ns 4.73 ns 4.94 ns
summary
G1-A
12.51x faster than G1-B
```
After this PR:
```
benchmark time (avg) iter/s (min … max) p75 p99 p995
--------------------------------------------------------------- -----------------------------
group G1
G1-A 3.85 ns/iter 259,822,096.0 (2.42 ns … 9.03 ns) 3.83 ns 4.62 ns 4.83 ns
G1-B 3.84 ns/iter 260,458,274.5 (3.55 ns … 7.05 ns) 3.83 ns 4.45 ns 4.7 ns
summary
G1-B
1x faster than G1-A
```
-rw-r--r-- | cli/tools/bench/mod.rs | 2 | ||||
-rw-r--r-- | cli/tools/bench/reporters.rs | 11 | ||||
-rw-r--r-- | tests/specs/bench/filter_group_header/__test__.jsonc | 5 | ||||
-rw-r--r-- | tests/specs/bench/filter_group_header/main.out | 13 | ||||
-rw-r--r-- | tests/specs/bench/filter_group_header/main.ts | 11 |
5 files changed, 31 insertions, 11 deletions
diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs index c640bea40..609dff282 100644 --- a/cli/tools/bench/mod.rs +++ b/cli/tools/bench/mod.rs @@ -233,7 +233,7 @@ async fn bench_specifier_inner( let benchmarks = if used_only { only } else { no_only }; let mut benchmarks = benchmarks .into_iter() - .filter(|(d, _)| filter.includes(&d.name) && !d.ignore) + .filter(|(d, _)| d.warmup || filter.includes(&d.name) && !d.ignore) .collect::<Vec<_>>(); let mut groups = IndexSet::<Option<String>>::new(); // make sure ungrouped benchmarks are placed above grouped diff --git a/cli/tools/bench/reporters.rs b/cli/tools/bench/reporters.rs index b09a73fc3..face54943 100644 --- a/cli/tools/bench/reporters.rs +++ b/cli/tools/bench/reporters.rs @@ -99,7 +99,6 @@ impl BenchReporter for JsonReporter { pub struct ConsoleReporter { name: String, show_output: bool, - has_ungrouped: bool, group: Option<String>, baseline: bool, group_measurements: Vec<(BenchDescription, BenchStats)>, @@ -114,7 +113,6 @@ impl ConsoleReporter { options: None, baseline: false, name: String::new(), - has_ungrouped: false, group_measurements: Vec::new(), } } @@ -173,18 +171,11 @@ impl BenchReporter for ConsoleReporter { self.name = desc.name.clone(); match &desc.group { - None => { - self.has_ungrouped = true; - } + None => {} Some(group) => { if self.group.is_none() || group != self.group.as_ref().unwrap() { self.report_group_summary(); - } - - if (self.group.is_none() && self.has_ungrouped) - || (self.group.is_some() && self.group_measurements.is_empty()) - { println!("{} {}", colors::gray("group"), colors::green(group)); } diff --git a/tests/specs/bench/filter_group_header/__test__.jsonc b/tests/specs/bench/filter_group_header/__test__.jsonc new file mode 100644 index 000000000..9453d5840 --- /dev/null +++ b/tests/specs/bench/filter_group_header/__test__.jsonc @@ -0,0 +1,5 @@ +// Regression test for https://github.com/denoland/deno/issues/23053 +{ + "args": "bench main.ts --filter=G", + "output": "main.out" +} diff --git a/tests/specs/bench/filter_group_header/main.out b/tests/specs/bench/filter_group_header/main.out new file mode 100644 index 000000000..e8b11299c --- /dev/null +++ b/tests/specs/bench/filter_group_header/main.out @@ -0,0 +1,13 @@ +Check [WILDCARD] +cpu: [WILDCARD] +runtime: [WILDCARD] + +[WILDCARD] +benchmark time (avg) iter/s (min … max) p75 p99 p995 +--------------------------------------------------------------- ----------------------------- + +group G1 +G1-B [WILDCARD] + +group G2 +G2-B [WILDCARD]
\ No newline at end of file diff --git a/tests/specs/bench/filter_group_header/main.ts b/tests/specs/bench/filter_group_header/main.ts new file mode 100644 index 000000000..16b244cf0 --- /dev/null +++ b/tests/specs/bench/filter_group_header/main.ts @@ -0,0 +1,11 @@ +Deno.bench({ + group: "G1", + name: "G1-B", + fn() {}, +}); + +Deno.bench({ + group: "G2", + name: "G2-B", + fn() {}, +}); |