benchmark: cleanup serde_json values being passed around (#9115)

Cargo.lock

@@ -2810,6 +2810,7 @@ dependencies = [
  "os_pipe",
  "pty",
  "regex",
+ "serde",
  "tempfile",
  "tokio",
  "tokio-rustls",

cli/bench/main.rs

@@ -1,7 +1,10 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 
-use deno_core::serde_json::{self, map::Map, Number, Value};
+use deno_core::serde_json::{self, Value};
+use serde::Serialize;
+use std::time::SystemTime;
 use std::{
+  collections::HashMap,
   convert::From,
   env, fs,
   path::PathBuf,
@@ -102,7 +105,10 @@ const EXEC_TIME_BENCHMARKS: &[(&str, &[&str], Option<i32>)] = &[
 
 const RESULT_KEYS: &[&str] =
   &["mean", "stddev", "user", "system", "min", "max"];
-fn run_exec_time(deno_exe: &PathBuf, target_dir: &PathBuf) -> Result<Value> {
+fn run_exec_time(
+  deno_exe: &PathBuf,
+  target_dir: &PathBuf,
+) -> Result<HashMap<String, HashMap<String, f64>>> {
   let hyperfine_exe = test_util::prebuilt_tool_path("hyperfine");
 
   let benchmark_file = target_dir.join("hyperfine_results.json");
@@ -143,7 +149,7 @@ fn run_exec_time(deno_exe: &PathBuf, target_dir: &PathBuf) -> Result<Value> {
     true,
   );
 
-  let mut results = Map::new();
+  let mut results = HashMap::<String, HashMap<String, f64>>::new();
   let hyperfine_results = read_json(benchmark_file)?;
   for ((name, _, _), data) in EXEC_TIME_BENCHMARKS.iter().zip(
     hyperfine_results
@@ -157,16 +163,15 @@ fn run_exec_time(deno_exe: &PathBuf, target_dir: &PathBuf) -> Result<Value> {
     let data = data.as_object().unwrap().clone();
     results.insert(
       name.to_string(),
-      Value::Object(
+      data
-        data
+        .into_iter()
-          .into_iter()
+        .filter(|(key, _)| RESULT_KEYS.contains(&key.as_str()))
-          .filter(|(key, _)| RESULT_KEYS.contains(&key.as_str()))
+        .map(|(key, val)| (key, val.as_f64().unwrap()))
-          .collect::<Map<String, Value>>(),
+        .collect(),
-      ),
     );
   }
 
-  Ok(Value::Object(results))
+  Ok(results)
 }
 
 fn rlib_size(target_dir: &std::path::Path, prefix: &str) -> u64 {
@@ -193,32 +198,29 @@ fn rlib_size(target_dir: &std::path::Path, prefix: &str) -> u64 {
 
 const BINARY_TARGET_FILES: &[&str] =
   &["CLI_SNAPSHOT.bin", "COMPILER_SNAPSHOT.bin"];
-fn get_binary_sizes(target_dir: &PathBuf) -> Result<Value> {
+fn get_binary_sizes(target_dir: &PathBuf) -> Result<HashMap<String, u64>> {
-  let mut sizes = Map::new();
+  let mut sizes = HashMap::<String, u64>::new();
-  let mut mtimes = std::collections::HashMap::new();
+  let mut mtimes = HashMap::<String, SystemTime>::new();
 
   sizes.insert(
     "deno".to_string(),
-    Value::Number(Number::from(test_util::deno_exe_path().metadata()?.len())),
+    test_util::deno_exe_path().metadata()?.len(),
   );
 
   // add up size for denort
   sizes.insert(
     "denort".to_string(),
-    Value::Number(Number::from(test_util::denort_exe_path().metadata()?.len())),
+    test_util::denort_exe_path().metadata()?.len(),
   );
 
   // add up size for everything in target/release/deps/libswc*
   let swc_size = rlib_size(&target_dir, "libswc");
   println!("swc {} bytes", swc_size);
-  sizes.insert("swc_rlib".to_string(), Value::Number(swc_size.into()));
+  sizes.insert("swc_rlib".to_string(), swc_size);
 
   let rusty_v8_size = rlib_size(&target_dir, "librusty_v8");
   println!("rusty_v8 {} bytes", rusty_v8_size);
-  sizes.insert(
+  sizes.insert("rusty_v8_rlib".to_string(), rusty_v8_size);
-    "rusty_v8_rlib".to_string(),
-    Value::Number(rusty_v8_size.into()),
-  );
 
   // Because cargo's OUT_DIR is not predictable, search the build tree for
   // snapshot related files.
@@ -244,18 +246,18 @@ fn get_binary_sizes(target_dir: &PathBuf) -> Result<Value> {
     }
 
     mtimes.insert(filename.clone(), file_mtime);
-    sizes.insert(filename, Value::Number(Number::from(meta.len())));
+    sizes.insert(filename, meta.len());
   }
 
-  Ok(Value::Object(sizes))
+  Ok(sizes)
 }
 
 const BUNDLES: &[(&str, &str)] = &[
   ("file_server", "./std/http/file_server.ts"),
   ("gist", "./std/examples/gist.ts"),
 ];
-fn bundle_benchmark(deno_exe: &PathBuf) -> Result<Value> {
+fn bundle_benchmark(deno_exe: &PathBuf) -> Result<HashMap<String, u64>> {
-  let mut sizes = Map::new();
+  let mut sizes = HashMap::<String, u64>::new();
 
   for (name, url) in BUNDLES {
     let path = format!("{}.bundle.js", name);
@@ -275,71 +277,48 @@ fn bundle_benchmark(deno_exe: &PathBuf) -> Result<Value> {
 
     let file = PathBuf::from(path);
     assert!(file.is_file());
-    sizes.insert(
+    sizes.insert(name.to_string(), file.metadata()?.len());
-      name.to_string(),
-      Value::Number(Number::from(file.metadata()?.len())),
-    );
     let _ = fs::remove_file(file);
   }
 
-  Ok(Value::Object(sizes))
+  Ok(sizes)
 }
 
-fn run_throughput(deno_exe: &PathBuf) -> Result<Value> {
+fn run_throughput(deno_exe: &PathBuf) -> Result<HashMap<String, f64>> {
-  let mut m = Map::new();
+  let mut m = HashMap::<String, f64>::new();
 
   m.insert("100M_tcp".to_string(), throughput::tcp(deno_exe, 100)?);
   m.insert("100M_cat".to_string(), throughput::cat(deno_exe, 100)?);
   m.insert("10M_tcp".to_string(), throughput::tcp(deno_exe, 10)?);
   m.insert("10M_cat".to_string(), throughput::cat(deno_exe, 10)?);
 
-  Ok(Value::Object(m))
+  Ok(m)
 }
 
-fn run_http(
+fn run_http(target_dir: &PathBuf, new_data: &mut BenchResult) -> Result<()> {
-  target_dir: &PathBuf,
-  new_data: &mut Map<String, Value>,
-) -> Result<()> {
   let stats = http::benchmark(target_dir)?;
 
-  new_data.insert(
+  new_data.req_per_sec = stats
-    "req_per_sec".to_string(),
+    .iter()
-    Value::Object(
+    .map(|(name, result)| (name.clone(), result.requests))
-      stats
+    .collect();
-        .iter()
-        .map(|(name, result)| {
-          (name.clone(), Value::Number(Number::from(result.requests)))
-        })
-        .collect::<Map<String, Value>>(),
-    ),
-  );
 
-  new_data.insert(
+  new_data.max_latency = stats
-    "max_latency".to_string(),
+    .iter()
-    Value::Object(
+    .map(|(name, result)| (name.clone(), result.latency))
-      stats
+    .collect();
-        .iter()
-        .map(|(name, result)| {
-          (
-            name.clone(),
-            Value::Number(Number::from_f64(result.latency).unwrap()),
-          )
-        })
-        .collect::<Map<String, Value>>(),
-    ),
-  );
 
   Ok(())
 }
 
 fn run_strace_benchmarks(
   deno_exe: &PathBuf,
-  new_data: &mut Map<String, Value>,
+  new_data: &mut BenchResult,
 ) -> Result<()> {
   use std::io::Read;
 
-  let mut thread_count = Map::new();
+  let mut thread_count = HashMap::<String, u64>::new();
-  let mut syscall_count = Map::new();
+  let mut syscall_count = HashMap::<String, u64>::new();
 
   for (name, args, _) in EXEC_TIME_BENCHMARKS {
     let mut file = tempfile::NamedTempFile::new()?;
@@ -361,26 +340,20 @@ fn run_strace_benchmarks(
     file.as_file_mut().read_to_string(&mut output)?;
 
     let strace_result = test_util::parse_strace_output(&output);
-    thread_count.insert(
+    let clone = strace_result.get("clone").map(|d| d.calls).unwrap_or(0);
-      name.to_string(),
+    let total = strace_result.get("total").unwrap().calls;
-      Value::Number(Number::from(
+    thread_count.insert(name.to_string(), clone);
-        strace_result.get("clone").map(|d| d.calls).unwrap_or(0) + 1,
+    syscall_count.insert(name.to_string(), total);
-      )),
-    );
-    syscall_count.insert(
-      name.to_string(),
-      Value::Number(Number::from(strace_result.get("total").unwrap().calls)),
-    );
   }
 
-  new_data.insert("thread_count".to_string(), Value::Object(thread_count));
+  new_data.thread_count = thread_count;
-  new_data.insert("syscall_count".to_string(), Value::Object(syscall_count));
+  new_data.syscall_count = syscall_count;
 
   Ok(())
 }
 
-fn run_max_mem_benchmark(deno_exe: &PathBuf) -> Result<Value> {
+fn run_max_mem_benchmark(deno_exe: &PathBuf) -> Result<HashMap<String, u64>> {
-  let mut results = Map::new();
+  let mut results = HashMap::<String, u64>::new();
 
   for (name, args, return_code) in EXEC_TIME_BENCHMARKS {
     let proc = Command::new("time")
@@ -396,13 +369,10 @@ fn run_max_mem_benchmark(deno_exe: &PathBuf) -> Result<Value> {
     }
     let out = String::from_utf8(proc_result.stderr)?;
 
-    results.insert(
+    results.insert(name.to_string(), test_util::parse_max_mem(&out).unwrap());
-      name.to_string(),
-      Value::Number(Number::from(test_util::parse_max_mem(&out).unwrap())),
-    );
   }
 
-  Ok(Value::Object(results))
+  Ok(results)
 }
 
 fn cargo_deps() -> usize {
@@ -420,6 +390,44 @@ fn cargo_deps() -> usize {
   count
 }
 
+#[derive(Serialize)]
+struct BenchResult {
+  created_at: String,
+  sha1: String,
+  binary_size: HashMap<String, u64>,
+  bundle_size: HashMap<String, u64>,
+  cargo_deps: usize,
+  // TODO(ry) The "benchmark" benchmark should actually be called "exec_time".
+  // When this is changed, the historical data in gh-pages branch needs to be
+  // changed too.
+  benchmark: HashMap<String, HashMap<String, f64>>,
+  throughput: HashMap<String, f64>,
+  max_memory: HashMap<String, u64>,
+  req_per_sec: HashMap<String, u64>,
+  max_latency: HashMap<String, f64>,
+  thread_count: HashMap<String, u64>,
+  syscall_count: HashMap<String, u64>,
+}
+
+impl BenchResult {
+  pub fn new() -> BenchResult {
+    BenchResult {
+      created_at: String::new(),
+      sha1: String::new(),
+      binary_size: HashMap::new(),
+      bundle_size: HashMap::new(),
+      cargo_deps: 0,
+      benchmark: HashMap::new(),
+      throughput: HashMap::new(),
+      max_memory: HashMap::new(),
+      req_per_sec: HashMap::new(),
+      max_latency: HashMap::new(),
+      thread_count: HashMap::new(),
+      syscall_count: HashMap::new(),
+    }
+  }
+}
+
 /*
  TODO(SyrupThinker)
  Switch to the #[bench] attribute once
@@ -439,52 +447,35 @@ fn main() -> Result<()> {
 
   env::set_current_dir(&test_util::root_path())?;
 
-  let mut new_data: Map<String, Value> = Map::new();
+  let mut new_data = BenchResult::new();
-  new_data.insert(
+  new_data.created_at =
-    "created_at".to_string(),
+    chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true);
-    Value::String(
+  new_data.sha1 = test_util::run_collect(
-      chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true),
+    &["git", "rev-parse", "HEAD"],
-    ),
+    None,
-  );
+    None,
-  new_data.insert(
+    None,
-    "sha1".to_string(),
+    true,
-    Value::String(
+  )
-      test_util::run_collect(
+  .0
-        &["git", "rev-parse", "HEAD"],
+  .trim()
-        None,
+  .to_string();
-        None,
-        None,
-        true,
-      )
-      .0
-      .trim()
-      .to_string(),
-    ),
-  );
 
-  new_data.insert("binary_size".to_string(), get_binary_sizes(&target_dir)?);
+  new_data.binary_size = get_binary_sizes(&target_dir)?;
-  new_data.insert("bundle_size".to_string(), bundle_benchmark(&deno_exe)?);
+  new_data.bundle_size = bundle_benchmark(&deno_exe)?;
-  new_data.insert("cargo_deps".to_string(), Value::Number(cargo_deps().into()));
+  new_data.cargo_deps = cargo_deps();
-
+  new_data.benchmark = run_exec_time(&deno_exe, &target_dir)?;
-  // TODO(ry) The "benchmark" benchmark should actually be called "exec_time".
-  // When this is changed, the historical data in gh-pages branch needs to be
-  // changed too.
-  new_data.insert(
-    "benchmark".to_string(),
-    run_exec_time(&deno_exe, &target_dir)?,
-  );
 
   // Cannot run throughput benchmark on windows because they don't have nc or
   // pipe.
   if cfg!(not(target_os = "windows")) {
-    new_data.insert("throughput".to_string(), run_throughput(&deno_exe)?);
+    new_data.throughput = run_throughput(&deno_exe)?;
     run_http(&target_dir, &mut new_data)?;
   }
 
   if cfg!(target_os = "linux") {
     run_strace_benchmarks(&deno_exe, &mut new_data)?;
-    new_data
+    new_data.max_memory = run_max_mem_benchmark(&deno_exe)?;
-      .insert("max_memory".to_string(), run_max_mem_benchmark(&deno_exe)?);
   }
 
   println!("===== <BENCHMARK RESULTS>");
@@ -492,7 +483,7 @@ fn main() -> Result<()> {
   println!("\n===== </BENCHMARK RESULTS>");
 
   if let Some(filename) = target_dir.join("bench.json").to_str() {
-    write_json(filename, &Value::Object(new_data))?;
+    write_json(filename, &serde_json::to_value(&new_data)?)?;
   } else {
     eprintln!("Cannot write bench.json, path is invalid");
   }

cli/bench/throughput.rs

@@ -1,7 +1,6 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 
 use super::Result;
-use deno_core::serde_json::{Number, Value};
 use std::{
   path::PathBuf,
   process::Command,
@@ -12,7 +11,7 @@ const MB: usize = 1024 * 1024;
 const SERVER_ADDR: &str = "0.0.0.0:4544";
 const CLIENT_ADDR: &str = "127.0.0.1 4544";
 
-pub(crate) fn cat(deno_exe: &PathBuf, megs: usize) -> Result<Value> {
+pub(crate) fn cat(deno_exe: &PathBuf, megs: usize) -> Result<f64> {
   let size = megs * MB;
   let shell_cmd = format!(
     "{} run --allow-read cli/tests/cat.ts /dev/zero | head -c {}",
@@ -26,12 +25,10 @@ pub(crate) fn cat(deno_exe: &PathBuf, megs: usize) -> Result<Value> {
   let _ = test_util::run_collect(cmd, None, None, None, true);
   let end = Instant::now();
 
-  Ok(Value::Number(
+  Ok((end - start).as_secs_f64())
-    Number::from_f64((end - start).as_secs_f64()).unwrap(),
-  ))
 }
 
-pub(crate) fn tcp(deno_exe: &PathBuf, megs: usize) -> Result<Value> {
+pub(crate) fn tcp(deno_exe: &PathBuf, megs: usize) -> Result<f64> {
   let size = megs * MB;
 
   // The GNU flavor of `nc` requires the `-N` flag to shutdown the network socket after EOF on stdin
@@ -66,7 +63,5 @@ pub(crate) fn tcp(deno_exe: &PathBuf, megs: usize) -> Result<Value> {
 
   echo_server.kill()?;
 
-  Ok(Value::Number(
+  Ok((end - start).as_secs_f64())
-    Number::from_f64((end - start).as_secs_f64()).unwrap(),
-  ))
 }

test_util/Cargo.toml

@@ -19,6 +19,7 @@ hyper = { version = "0.14.2", features = ["server", "http1", "runtime"] }
 lazy_static = "1.4.0"
 os_pipe = "0.9.2"
 regex = "1.4.3"
+serde = { version = "1.0.116", features = ["derive"] }
 tempfile = "3.1.0"
 tokio = { version = "1.0.1", features = ["full"] }
 tokio-rustls = "0.22.0"

test_util/src/lib.rs

@@ -21,6 +21,7 @@ use os_pipe::pipe;
 #[cfg(unix)]
 pub use pty;
 use regex::Regex;
+use serde::Serialize;
 use std::collections::HashMap;
 use std::convert::Infallible;
 use std::env;
@@ -1366,7 +1367,7 @@ pub fn parse_wrk_output(output: &str) -> WrkOutput {
   }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone, Serialize)]
 pub struct StraceOutput {
   pub percent_time: f64,
   pub seconds: f64,