summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNathan Whitaker <17734409+nathanwhit@users.noreply.github.com>2024-03-26 09:19:24 -0700
committerGitHub <noreply@github.com>2024-03-26 09:19:24 -0700
commita2a537e196360c5e44821f3bff901bd478ab8156 (patch)
tree39bb427d5abb7b75d2ed9eba9c8655728985f99c
parent89aa6d5cf6ae76e3698587923453f535b9af81af (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.rs2
-rw-r--r--cli/tools/bench/reporters.rs11
-rw-r--r--tests/specs/bench/filter_group_header/__test__.jsonc5
-rw-r--r--tests/specs/bench/filter_group_header/main.out13
-rw-r--r--tests/specs/bench/filter_group_header/main.ts11
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() {},
+});