From a2a537e196360c5e44821f3bff901bd478ab8156 Mon Sep 17 00:00:00 2001
From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com>
Date: Tue, 26 Mar 2024 09:19:24 -0700
Subject: [PATCH] fix(bench): Fix group header printing logic + don't filter
 out the warmup benchmark (#23083)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
```
---
 cli/tools/bench/mod.rs                              |  2 +-
 cli/tools/bench/reporters.rs                        | 11 +----------
 .../specs/bench/filter_group_header/__test__.jsonc  |  5 +++++
 tests/specs/bench/filter_group_header/main.out      | 13 +++++++++++++
 tests/specs/bench/filter_group_header/main.ts       | 11 +++++++++++
 5 files changed, 31 insertions(+), 11 deletions(-)
 create mode 100644 tests/specs/bench/filter_group_header/__test__.jsonc
 create mode 100644 tests/specs/bench/filter_group_header/main.out
 create mode 100644 tests/specs/bench/filter_group_header/main.ts

diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs
index c640bea40b..609dff282f 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 b09a73fc37..face54943f 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 0000000000..9453d58409
--- /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 0000000000..e8b11299ca
--- /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 0000000000..16b244cf06
--- /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() {},
+});