Add JSON reporter for "deno bench" subcommand

[bartlomieju]

Hopefully we could do it for v1.22 to allow stabilization of "Deno.bench()" API.

[Im-Beast]

Is there any progress on this?
This would come in quite handy for me to generate markdown tables for each commit to analyze progressional speed difference

[bartlomieju]

No progress so far. PRs and help would be highly appreciated.

[Im-Beast]

Gotta wait for someone to take the stick, I'm not familiar with Rust at all 😅

[Lurk]

That is the feature I need, and I will be happy to spend some evenings implementing it.

I need some more input.

For me, a JSON file with the BenchReport in it would be enough. Is that what everybody wants?

The way to switch between reporters deno bench --reporter={console,json} where the default will be the console.

Do we want to specify the benchmark results location? Something like optional --file-reporter-location=./benchmarks

[jsejcksn]

The way to switch between reporters deno bench --reporter={console,json} where the default will be the console.

Do we want to specify the benchmark results location? Something like optional --file-reporter-location=./benchmarks

+1 on the console suggestion, but I prefer the label stdout as it is more descriptive of the actual output stream — which could be used in a piped operation to another process or redirected to a file, for example:

# pipe to another deno program to parse the JSON and analyze the data:
deno bench --reporter=json | deno run analyze_bench_results.ts

# or redirect to a file on disk for future use:
deno bench --reporter=json > bench_data.json

[Lurk]

@jsejcksn Thank you for the input.

On console vs stdout. Maybe there is confusion in terminology. Deno has one reporter ConsoleReporter which produces beautiful console output.

My idea is to add one more reporter which will produce JSON output. And I like the idea of outputting it to stdout instead of writing it to some predefined file.

[jsejcksn]

My idea is to add one more reporter which will produce JSON output. And I like the idea of outputting it to stdout instead of writing it to some predefined file.

@Lurk According to the examples in the manual, the benchmarking tool currently always outputs to the stdout stream. If you were suggesting allowing for the optional writing to other destinations (e.g. a file on disk), then I was simply suggesting using the label stdout in favor of console when targeting stdout.

[Lurk]

@jsejcksn There will be no changes to the output destination, only its format.

Updated plan:

• add new benchmark reporter JSONReporter, which outputs to stdout serialized to JSON BenchReport
• add new CLI flag --reporter={console,json} to switch between the existing reporter and the new one

Example usage:

# pipe to another deno program to parse the JSON and analyze the data:
deno bench --reporter=json | deno run analyze_bench_results.ts

# or redirect to a file on disk for future use:
deno bench --reporter=json > bench_data.json

[iuioiua]

Why not just --json like the other subcommands? Am I missing something here?

[jsejcksn]

Why not just --json like the other subcommands? Am I missing something here?

@iuioiua I think the idea is to allow for additional future reporter serialization formats (e.g. CSV). Having a dedicated boolean argument for every future reporter could become unwieldy — and trying to use them in combination wouldn't make sense (e.g. deno bench --csv --json 👈 What should this be: CSV or JSON?).

[iuioiua]

I'm unaware of any other subcommands supporting CSV output. Are there any plans to add support for CSV output for any subcommands? If not, I don't see why deno bench should support it while others don't.

[Lurk]

@iuioiua
As @jsejcksn mentioned, the idea was not to limit ourselves to only JSON output.

The internal implementation of reporters suggests that there can be multiple implementations. I want to add JSON, someone else will want to add CSV, YAML, XML, or whatever else is out there. If we add just a boolean flag, we are adding a breaking change for the future.

UPD.
checked all other subcommand (lint, doc) and there is a --json flag.

I still like --reporter=json more, but to keep it consistent, --json is not a bad idea.

[micheledurante]

I would also be interested in the --json flag

[bartlomieju]

Does anyone have a proposed format for the JSON output?

[iuioiua]

Taken from the manual, this

$ deno bench --unstable time_bench.ts
cpu: Apple M1 Max
runtime: deno 1.21.0 (aarch64-apple-darwin)

file:///dev/deno/time_bench.ts
benchmark              time (avg)             (min … max)       p75       p99      p995
--------------------------------------------------------- -----------------------------
Date.now()         125.24 ns/iter (118.98 ns … 559.95 ns) 123.62 ns 150.69 ns 156.63 ns
performance.now()    2.67 µs/iter     (2.64 µs … 2.82 µs)   2.67 µs   2.82 µs   2.82 µs

summary
  Date.now()
   21.29x times faster than performance.now()

could look like this where the time unit is seconds.

[
  {
    "cpu": "Apple M1 Max",
    "runtime": "deno 1.21.0 (aarch64-apple-darwin)",
    "file": "/dev/deno/time_bench.ts",
    "group": "timing",
    "results": [
      {
        "name": "Date.now()",
        "avg": 125e-9,
        "min": 118.98e-9,
        "max": 559.95e-9,
        "p75": 123.62e-9,
        "p99": 150.69e-9,
        "p995": 156.63e-9
      },
      {
        "name": "performance.now()",
        "avg": 2.67e-6,
        "min": 2.64e-6,
        "max": 2.82e-6,
        "p75": 2.67e-6,
        "p99": 2.82e-6,
        "p995": 2.82e-6
      }
    ]
  }
]

[jsejcksn]

I think that's in the right direction, and here is some feedback:

the time unit is seconds

The SI unit of time is seconds, and without some kind of self-documenting unit, seconds are very reasonable. Another reference is UNIX time.

"file": "/dev/deno/time_bench.ts",

Is it safe to assume that remote URLs are supported? If so, then perhaps the key could be something like url and include the full file: URL:

"url": "file:///dev/deno/time_bench.ts",

[Lurk]

The SI unit of time is seconds, and without some kind of self-documenting unit, seconds are very reasonable. Another reference is UNIX time.

IMO the second is too big as a standard unit of measurement for benchmarking. In JavaScript land, everything is millisecond Date.now() or microseconds performance.now().

If we look at the example output @iuioiua gave

...
Date.now()         125.24 ns/iter (118.98 ns … 559.95 ns) 123.62 ns 150.69 ns 156.63 ns
...

Even a nanosecond precision is not enough.

9_007_199_254_740_991 is the max value we can safely use in JSON.

9_007_199_254_740_991 nanoseconds are roughly 104 days, which sounds like a pretty reasonable number for benchmarking.

I would say we should use nanoseconds as a unit of measurement. It can be easily upscaled to seconds or even days if someone needs it, and at the same time gives more precision which is important when you are in a measuring contest.

[iuioiua]

IMO the second is too big as a standard unit of measurement for benchmarking. In JavaScript land, everything is millisecond Date.now() or microseconds performance.now().

Actually, milliseconds is likely the best unit because it's consistent with other time-related APIs.

Precision isn't an issue thanks to decimals.

[Lurk]

Just to clarify

Nanoseconds:

{
    "name": "Date.now()",
    "avg": 125.24,
    "min": 118.98,
    "max": 559.95,
    "p75": 123.62,
    "p99": 150.69,
    "p995": 156.63
}

Milliseconds:

{
    "name": "Date.now()",
    "avg": 0.00012524,
    "min": 0.00011898,
    "max": 0.00055995,
    "p75": 0.00012362,
    "p99": 0.00015069,
    "p995": 0.00015663
}

[iuioiua]

It's also worth noting that in most cases, the JSON data printed will be consumed by some script or similar and doesn't need to be human-readable. So the number of significant figures isn't super important.

[Lurk]

@bartlomieju Should we close this one?

[bartlomieju]

Yeah, thanks for the ping!