Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bench): Fix group header printing logic + don't filter out the warmup benchmark #23083

Merged
merged 3 commits into from Mar 26, 2024

Conversation

nathanwhit
Copy link
Member

@nathanwhit nathanwhit commented Mar 26, 2024

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

deno bench main.js --filter="G"
// 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

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@nathanwhit nathanwhit merged commit a2a537e into denoland:main Mar 26, 2024
17 checks passed
@nathanwhit nathanwhit deleted the deno-bench-header branch March 26, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deno bench omits first group header if --filter is used
2 participants