feat(bench): Add JSON reporter for "deno bench" subcommand #17595

Lurk · 2023-01-30T16:05:27Z

Initial discussion in #14385

In this pull request:

New --json flag to bench subcommand.

time unit: nanoseconds.

Output format example:

[
  {
    "runtime": "Deno/1.28.3 x86_64-apple-darwin",
    "cpu": "Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz",
    "origin": "file:///Users/serhiy.barhamon/tmp/deno/test_ffi/tests/bench.js",
    "group": null,
    "name": "Deno.UnsafePointerView#getUint32",
    "baseline": false,
    "result": {
      "ok": {
        "n": 49,
        "min": 1251.9348,
        "max": 1441.2696,
        "avg": 1308.7523755102038,
        "p75": 1324.1055,
        "p99": 1441.2696,
        "p995": 1441.2696,
        "p999": 1441.2696
      }
    }
  },
  {
    "runtime": "Deno/1.28.3 x86_64-apple-darwin",
    "cpu": "Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz",
    "origin": "file:///Users/serhiy.barhamon/tmp/deno/test_ffi/tests/bench.js",
    "group": null,
    "name": "nop()",
    "baseline": false,
    "result": {
      "failed": {
        "name": "Error",
        "message": "test",
        "stack": "Error: test\n    at file:///Users/serhiy.barhamon/tmp/deno/test_ffi/tests/bench.js:241:11/n    at benchMeasure (deno:cli/js/40_testing.js:898:9)\n    at runBench (deno:cli/js/40_testing.js:1009:27)\n    at runBenchmarks (deno:cli/js/40_testing.js:1146:33)",
        "cause": null,
        "exceptionMessage": "Uncaught Error: test",
        "frames": [
          {
            "typeName": null,
            "functionName": null,
            "methodName": null,
            "fileName": "file:///Users/serhiy.barhamon/tmp/deno/test_ffi/tests/bench.js",
            "lineNumber": 241,
            "columnNumber": 11,
            "evalOrigin": null,
            "isToplevel": true,
            "isEval": false,
            "isNative": false,
            "isConstructor": false,
            "isAsync": false,
            "isPromiseAll": false,
            "promiseIndex": null
          },
          {
            "typeName": null,
            "functionName": "benchMeasure",
            "methodName": null,
            "fileName": "deno:cli/js/40_testing.js",
            "lineNumber": 898,
            "columnNumber": 9,
            "evalOrigin": null,
            "isToplevel": true,
            "isEval": false,
            "isNative": false,
            "isConstructor": false,
            "isAsync": false,
            "isPromiseAll": false,
            "promiseIndex": null
          },
          {
            "typeName": null,
            "functionName": "runBench",
            "methodName": null,
            "fileName": "deno:cli/js/40_testing.js",
            "lineNumber": 1009,
            "columnNumber": 27,
            "evalOrigin": null,
            "isToplevel": true,
            "isEval": false,
            "isNative": false,
            "isConstructor": false,
            "isAsync": false,
            "isPromiseAll": false,
            "promiseIndex": null
          },
          {
            "typeName": null,
            "functionName": "runBenchmarks",
            "methodName": null,
            "fileName": "deno:cli/js/40_testing.js",
            "lineNumber": 1146,
            "columnNumber": 33,
            "evalOrigin": null,
            "isToplevel": true,
            "isEval": false,
            "isNative": false,
            "isConstructor": false,
            "isAsync": false,
            "isPromiseAll": false,
            "promiseIndex": null
          }
        ],
        "sourceLine": "    throw new Error(\"test\");",
        "sourceLineFrameIndex": 0,
        "aggregated": null
      }
    }
]

Added the `json` flag, copypasted ConsoleReporter to JsonReporter and shuffled code arround

output format: ``` [ { "runtime": "Deno/1.28.3 x86_64-apple-darwin", "cpu": "Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz", "origin": "file:///Users/serhiy.barhamon/tmp/deno/test_ffi/tests/bench.js", "group": null, "name": "Deno.UnsafePointerView#getUint32", "baseline": false, "result": { "ok": { "n": 49, "min": 1251.9348, "max": 1441.2696, "avg": 1308.7523755102038, "p75": 1324.1055, "p99": 1441.2696, "p995": 1441.2696, "p999": 1441.2696 } } } ] ``` time unit: nanoseconds

CLAassistant · 2023-01-30T16:05:37Z

All committers have signed the CLA.

Lurk · 2023-01-31T17:41:28Z

I think I am done.
I don't like how the failed result looks, but I also do not have any better Idea.

aapoalas

Aside from the output format, LGTM. Core team should probably chime in on if the output format should be a flat list like it is now, or a structured object that aligns a bit more with the semantics of the bench command.

aapoalas · 2023-02-11T10:48:22Z

cli/tools/bench.rs

@@ -123,6 +132,74 @@ pub trait BenchReporter {
  fn report_result(&mut self, desc: &BenchDescription, result: &BenchResult);
 }

+#[derive(Debug, Serialize)]
+struct JsonReporterResult {


Bikeshedding: I would perhaps split up the formatting a bit; now each result field includes the runtime and CPU information, and groups are only an entry in the result data and not actually groups as the name might suggest.

I might rather go with something like:

{ "runtime": "...", "cpu": "...", "groups": [ { "name": null, "results": [ { "name": "nop()", "origin": "file.js", "baseline": false, "result": { "ok": { } } } ] } ] }

aapoalas · 2023-02-11T10:53:22Z

cli/tools/bench.rs

+
+  fn report_result(&mut self, desc: &BenchDescription, result: &BenchResult) {
+    self.0.push(JsonReporterResult::new(
+      desc.origin.clone(),


doc JSON output uses location.filename (together with location.line and location.col, and the error stack uses fileName. Maybe align with either or?

Lurk · 2023-02-11T11:10:07Z

@aapoalas On the output format.
I choose a flat structure because IMO that will be a usual way to consume results:

Run bench on CI
Save results to the Elastic/Prometheus/etc
Query that data to build charts/send alerts/etc

A flat structure is much better in use cases like that.

But I am ready to change it if my assumptions are wrong.

aapoalas · 2023-02-11T11:15:56Z

cli/args/flags.rs

+    .arg(
+      Arg::new("json")
+        .long("json")
+        .help("Output benchmark result in JSON format")


Add UNSTABLE: to the help string for now.

aapoalas · 2023-02-11T11:16:57Z

cli/args/flags.rs

@@ -751,6 +758,10 @@ and report results to standard output:

  deno bench src/fetch_bench.ts src/signal_bench.ts

+Output benchmark result in JSON format:


This isn't really needed I think, the --help will print out the individual args either way so this will just be redundant information.

aapoalas · 2023-02-11T11:38:08Z

@aapoalas On the output format. I choose a flat structure because IMO that will be a usual way to consume results:

Run bench on CI

Save results to the Elastic/Prometheus/etc

Query that data to build charts/send alerts/etc

A flat structure is much better in use cases like that.

But I am ready to change it if my assumptions are wrong.

Sounds like a good enough reason to go with your choice format. I kind of would expect the workflow to go otherwise the same but with some processing step between running and saving to a database, where massaging would be done and at that point it might be useful to have the data already eg. grouped up so the massaging wouldn't need to start with a "divide into groups" step.

aapoalas

LGTM. We can iterate on the exact format down the line.

Serhiy Barhamon added 3 commits December 14, 2022 20:52

#14385 WIP

ad337d3

Added the `json` flag, copypasted ConsoleReporter to JsonReporter and shuffled code arround

Merge branch 'denoland:main' into main

be96e7e

Merge branch 'main' into main

92e8ae9

Lurk changed the title ~~feat(bench): Add JSON reporter for "deno bench" subcommand~~ [WIP] feat(bench): Add JSON reporter for "deno bench" subcommand Jan 30, 2023

#14385 fix merge conflict mess

1d7be61

Lurk changed the title ~~[WIP] feat(bench): Add JSON reporter for "deno bench" subcommand~~ feat(bench): Add JSON reporter for "deno bench" subcommand Jan 31, 2023

bartlomieju added this to the 1.31 milestone Feb 2, 2023

Serhiy Barhamon added 5 commits February 3, 2023 12:11

Merge branch 'denoland:main' into main

f3fccf1

Fix tests

a06b5e1

make clippy happy

e8499b6

Merge branch 'main' into main

e65cbec

Merge branch 'main' into main

b06c993

petamoriken mentioned this pull request Feb 4, 2023

update(no-deprecated-deno-api): deprecate Deno.run denoland/deno_lint#1128

Merged

Serhiy Barhamon added 6 commits February 5, 2023 12:23

Merge branch 'main' into main

cae37e6

Merge branch 'main' into main

905f428

Merge branch 'main' into main

45ab517

Merge branch 'main' into main

f0c90c1

Merge branch 'main' into main

302a979

Merge branch 'main' into main

149b97a

aapoalas reviewed Feb 11, 2023

View reviewed changes

aapoalas requested review from dsherret and bartlomieju February 11, 2023 11:04

aapoalas reviewed Feb 11, 2023

View reviewed changes

Merge branch 'main' into main

0dae393

Serhiy Barhamon and others added 2 commits February 11, 2023 15:42

fix doc args

c7e9729

Merge branch 'main' into main

622daf0

aapoalas enabled auto-merge (squash) February 12, 2023 17:16

aapoalas approved these changes Feb 12, 2023

View reviewed changes

aapoalas merged commit fc843d0 into denoland:main Feb 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(bench): Add JSON reporter for "deno bench" subcommand #17595

feat(bench): Add JSON reporter for "deno bench" subcommand #17595

Lurk commented Jan 30, 2023

CLAassistant commented Jan 30, 2023 •

edited

Loading

Lurk commented Jan 31, 2023

aapoalas left a comment

aapoalas Feb 11, 2023

aapoalas Feb 11, 2023

Lurk commented Feb 11, 2023 •

edited

Loading

aapoalas Feb 11, 2023

Lurk Feb 11, 2023

aapoalas Feb 11, 2023

Lurk Feb 11, 2023

aapoalas commented Feb 11, 2023

aapoalas left a comment

		@@ -751,6 +758,10 @@ and report results to standard output:

		deno bench src/fetch_bench.ts src/signal_bench.ts

		Output benchmark result in JSON format:

feat(bench): Add JSON reporter for "deno bench" subcommand #17595

feat(bench): Add JSON reporter for "deno bench" subcommand #17595

Conversation

Lurk commented Jan 30, 2023

CLAassistant commented Jan 30, 2023 • edited Loading

Lurk commented Jan 31, 2023

aapoalas left a comment

Choose a reason for hiding this comment

aapoalas Feb 11, 2023

Choose a reason for hiding this comment

aapoalas Feb 11, 2023

Choose a reason for hiding this comment

Lurk commented Feb 11, 2023 • edited Loading

aapoalas Feb 11, 2023

Choose a reason for hiding this comment

Lurk Feb 11, 2023

Choose a reason for hiding this comment

aapoalas Feb 11, 2023

Choose a reason for hiding this comment

Lurk Feb 11, 2023

Choose a reason for hiding this comment

aapoalas commented Feb 11, 2023

aapoalas left a comment

Choose a reason for hiding this comment

CLAassistant commented Jan 30, 2023 •

edited

Loading

Lurk commented Feb 11, 2023 •

edited

Loading