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

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

Merged
merged 19 commits into from
Feb 12, 2023
13 changes: 13 additions & 0 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub struct FileFlags {
pub struct BenchFlags {
pub files: FileFlags,
pub filter: Option<String>,
pub json: bool,
}

#[derive(Clone, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -717,6 +718,12 @@ fn clap_root(version: &str) -> Command {
fn bench_subcommand<'a>() -> Command<'a> {
runtime_args(Command::new("bench"), true, false)
.trailing_var_arg(true)
.arg(
Arg::new("json")
.long("json")
.help("UNSTABLE: Output benchmark result in JSON format")
.takes_value(false),
)
.arg(
Arg::new("ignore")
.long("ignore")
Expand Down Expand Up @@ -2325,6 +2332,8 @@ fn bench_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
// interactive prompts, unless done by user code
flags.no_prompt = true;

let json = matches.is_present("json");

let ignore = match matches.values_of("ignore") {
Some(f) => f.map(PathBuf::from).collect(),
None => vec![],
Expand Down Expand Up @@ -2359,6 +2368,7 @@ fn bench_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
flags.subcommand = DenoSubcommand::Bench(BenchFlags {
files: FileFlags { include, ignore },
filter,
json,
});
}

Expand Down Expand Up @@ -6535,6 +6545,7 @@ mod tests {
let r = flags_from_vec(svec![
"deno",
"bench",
"--json",
"--unstable",
"--filter",
"- foo",
Expand All @@ -6552,6 +6563,7 @@ mod tests {
Flags {
subcommand: DenoSubcommand::Bench(BenchFlags {
filter: Some("- foo".to_string()),
json: true,
files: FileFlags {
include: vec![PathBuf::from("dir1/"), PathBuf::from("dir2/")],
ignore: vec![],
Expand All @@ -6576,6 +6588,7 @@ mod tests {
Flags {
subcommand: DenoSubcommand::Bench(BenchFlags {
filter: None,
json: false,
files: FileFlags {
include: vec![],
ignore: vec![],
Expand Down
2 changes: 2 additions & 0 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ impl CacheSetting {
pub struct BenchOptions {
pub files: FilesConfig,
pub filter: Option<String>,
pub json: bool,
}

impl BenchOptions {
Expand All @@ -119,6 +120,7 @@ impl BenchOptions {
Some(bench_flags.files),
),
filter: bench_flags.filter,
json: bench_flags.json,
})
}
}
Expand Down
90 changes: 86 additions & 4 deletions cli/tools/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::args::BenchOptions;
use crate::args::CliOptions;
use crate::args::TypeCheckMode;
use crate::colors;
use crate::display::write_json_to_stdout;
use crate::graph_util::graph_valid_with_cli_options;
use crate::ops;
use crate::proc_state::ProcState;
Expand All @@ -13,6 +14,7 @@ use crate::util::file_watcher;
use crate::util::file_watcher::ResolutionResult;
use crate::util::fs::collect_specifiers;
use crate::util::path::is_supported_ext;
use crate::version::get_user_agent;
use crate::worker::create_main_worker_for_test_or_bench;

use deno_core::error::generic_error;
Expand Down Expand Up @@ -41,6 +43,7 @@ use tokio::sync::mpsc::UnboundedSender;
#[derive(Debug, Clone)]
struct BenchSpecifierOptions {
filter: TestFilter,
json: bool,
}

#[derive(Debug, Clone, Eq, PartialEq, Deserialize)]
Expand All @@ -62,7 +65,7 @@ pub enum BenchEvent {
Result(usize, BenchResult),
}

#[derive(Debug, Clone, Deserialize)]
#[derive(Debug, Clone, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub enum BenchResult {
Ok(BenchStats),
Expand Down Expand Up @@ -109,7 +112,13 @@ impl BenchReport {
}
}

fn create_reporter(show_output: bool) -> Box<dyn BenchReporter + Send> {
fn create_reporter(
show_output: bool,
json: bool,
) -> Box<dyn BenchReporter + Send> {
if json {
return Box::new(JsonReporter::new());
}
Box::new(ConsoleReporter::new(show_output))
}

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

#[derive(Debug, Serialize)]
struct JsonReporterResult {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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": { } }
        }
      ]
    }
  ]
}

runtime: String,
cpu: String,
origin: String,
group: Option<String>,
name: String,
baseline: bool,
result: BenchResult,
}

impl JsonReporterResult {
fn new(
origin: String,
group: Option<String>,
name: String,
baseline: bool,
result: BenchResult,
) -> Self {
Self {
runtime: format!("{} {}", get_user_agent(), env!("TARGET")),
cpu: mitata::cpu::name(),
origin,
group,
name,
baseline,
result,
}
}
}

#[derive(Debug, Serialize)]
struct JsonReporter(Vec<JsonReporterResult>);
impl JsonReporter {
fn new() -> Self {
Self(vec![])
}
}

impl BenchReporter for JsonReporter {
fn report_group_summary(&mut self) {}
#[cold]
fn report_plan(&mut self, _plan: &BenchPlan) {}

fn report_end(&mut self, _report: &BenchReport) {
match write_json_to_stdout(self) {
Ok(_) => (),
Err(e) => println!("{e}"),
}
}

fn report_register(&mut self, _desc: &BenchDescription) {}

fn report_wait(&mut self, _desc: &BenchDescription) {}

fn report_output(&mut self, _output: &str) {}

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

Choose a reason for hiding this comment

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

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

desc.group.clone(),
desc.name.clone(),
desc.baseline,
result.clone(),
));
}
}

struct ConsoleReporter {
name: String,
show_output: bool,
Expand Down Expand Up @@ -376,12 +453,14 @@ async fn bench_specifiers(

let (sender, mut receiver) = unbounded_channel::<BenchEvent>();

let option_for_handles = options.clone();

let join_handles = specifiers.into_iter().map(move |specifier| {
let ps = ps.clone();
let permissions = permissions.clone();
let specifier = specifier;
let sender = sender.clone();
let options = options.clone();
let options = option_for_handles.clone();

tokio::task::spawn_blocking(move || {
let future = bench_specifier(ps, permissions, specifier, sender, options);
Expand All @@ -398,7 +477,8 @@ async fn bench_specifiers(
tokio::task::spawn(async move {
let mut used_only = false;
let mut report = BenchReport::new();
let mut reporter = create_reporter(log_level != Some(Level::Error));
let mut reporter =
create_reporter(log_level != Some(Level::Error), options.json);
let mut benches = IndexMap::new();

while let Some(event) = receiver.recv().await {
Expand Down Expand Up @@ -509,6 +589,7 @@ pub async fn run_benchmarks(
specifiers,
BenchSpecifierOptions {
filter: TestFilter::from_flag(&bench_options.filter),
json: bench_options.json,
},
)
.await?;
Expand Down Expand Up @@ -658,6 +739,7 @@ pub async fn run_benchmarks_with_watch(
specifiers,
BenchSpecifierOptions {
filter: TestFilter::from_flag(&bench_options.filter),
json: bench_options.json,
},
)
.await?;
Expand Down