refactor(bench): align ops to testing ops (#23038)

Internal refactor that changes how we use ops in `deno bench`
subcommand.

This brings it in line to what we do in `deno test` subcommand.

cli/js/40_bench.js

@@ -9,7 +9,12 @@ import {
 } from "ext:cli/40_test_common.js";
 import { Console } from "ext:deno_console/01_console.js";
 import { setExitHandler } from "ext:runtime/30_os.js";
-const ops = core.ops;
+const {
+  op_register_bench,
+  op_bench_get_origin,
+  op_dispatch_bench_event,
+  op_bench_now,
+} = core.ops;
 const {
   ArrayPrototypePush,
   Error,
@@ -30,6 +35,12 @@ let currentBenchUserExplicitEnd = null;
 
 let registeredWarmupBench = false;
 
+const registerBenchIdRetBuf = new Uint32Array(1);
+const registerBenchIdRetBufU8 = new Uint8Array(registerBenchIdRetBuf.buffer);
+
+// As long as we're using one isolate per test, we can cache the origin since it won't change
+let cachedOrigin = undefined;
+
 // Main bench function provided by Deno.
 function bench(
   nameOrFnOrOptions,
@@ -49,10 +60,22 @@ function bench(
       permissions: null,
       warmup: true,
     };
+    if (cachedOrigin == undefined) {
+      cachedOrigin = op_bench_get_origin();
+    }
     warmupBenchDesc.fn = wrapBenchmark(warmupBenchDesc);
-    const { id, origin } = ops.op_register_bench(warmupBenchDesc);
-    warmupBenchDesc.id = id;
-    warmupBenchDesc.origin = origin;
+    op_register_bench(
+      warmupBenchDesc.fn,
+      warmupBenchDesc.name,
+      warmupBenchDesc.baseline,
+      warmupBenchDesc.group,
+      warmupBenchDesc.ignore,
+      warmupBenchDesc.only,
+      warmupBenchDesc.warmup,
+      registerBenchIdRetBufU8,
+    );
+    warmupBenchDesc.id = registerBenchIdRetBufU8[0];
+    warmupBenchDesc.origin = cachedOrigin;
   }
 
   let benchDesc;
@@ -139,10 +162,21 @@ function bench(
   benchDesc.fn = wrapBenchmark(benchDesc);
   benchDesc.warmup = false;
   benchDesc.name = escapeName(benchDesc.name);
-
-  const { id, origin } = ops.op_register_bench(benchDesc);
-  benchDesc.id = id;
-  benchDesc.origin = origin;
+  if (cachedOrigin == undefined) {
+    cachedOrigin = op_bench_get_origin();
+  }
+  op_register_bench(
+    benchDesc.fn,
+    benchDesc.name,
+    benchDesc.baseline,
+    benchDesc.group,
+    benchDesc.ignore,
+    benchDesc.only,
+    false,
+    registerBenchIdRetBufU8,
+  );
+  benchDesc.id = registerBenchIdRetBufU8[0];
+  benchDesc.origin = cachedOrigin;
 }
 
 function compareMeasurements(a, b) {
@@ -381,7 +415,7 @@ function wrapBenchmark(desc) {
 
     try {
       globalThis.console = new Console((s) => {
-        ops.op_dispatch_bench_event({ output: s });
+        op_dispatch_bench_event({ output: s });
       });
 
       if (desc.permissions) {
@@ -420,7 +454,7 @@ function wrapBenchmark(desc) {
 }
 
 function benchNow() {
-  return ops.op_bench_now();
+  return op_bench_now();
 }
 
 globalThis.Deno.bench = bench;

cli/ops/bench.rs

@@ -5,17 +5,15 @@ use std::sync::atomic::Ordering;
 use std::time;
 
 use deno_core::error::generic_error;
+use deno_core::error::type_error;
 use deno_core::error::AnyError;
 use deno_core::op2;
-use deno_core::serde_v8;
 use deno_core::v8;
 use deno_core::ModuleSpecifier;
 use deno_core::OpState;
 use deno_runtime::permissions::create_child_permissions;
 use deno_runtime::permissions::ChildPermissionsArg;
 use deno_runtime::permissions::PermissionsContainer;
-use serde::Deserialize;
-use serde::Serialize;
 use tokio::sync::mpsc::UnboundedSender;
 use uuid::Uuid;
 
@@ -32,6 +30,7 @@ deno_core::extension!(deno_bench,
     op_pledge_test_permissions,
     op_restore_test_permissions,
     op_register_bench,
+    op_bench_get_origin,
     op_dispatch_bench_event,
     op_bench_now,
   ],
@@ -44,6 +43,12 @@ deno_core::extension!(deno_bench,
   },
 );
 
+#[op2]
+#[string]
+fn op_bench_get_origin(state: &mut OpState) -> String {
+  state.borrow::<ModuleSpecifier>().to_string()
+}
+
 #[derive(Clone)]
 struct PermissionsHolder(Uuid, PermissionsContainer);
 
@@ -94,57 +99,47 @@ pub fn op_restore_test_permissions(
   }
 }
 
-#[derive(Deserialize)]
-#[serde(rename_all = "camelCase")]
-struct BenchInfo<'s> {
-  #[serde(rename = "fn")]
-  function: serde_v8::Value<'s>,
-  name: String,
-  baseline: bool,
-  group: Option<String>,
-  ignore: bool,
-  only: bool,
-  #[serde(default)]
-  warmup: bool,
-}
-
-#[derive(Debug, Serialize)]
-#[serde(rename_all = "camelCase")]
-struct BenchRegisterResult {
-  id: usize,
-  origin: String,
-}
-
 static NEXT_ID: AtomicUsize = AtomicUsize::new(0);
 
+#[allow(clippy::too_many_arguments)]
 #[op2]
-#[serde]
-fn op_register_bench<'a>(
-  scope: &mut v8::HandleScope<'a>,
+fn op_register_bench(
   state: &mut OpState,
-  #[serde] info: BenchInfo<'a>,
-) -> Result<BenchRegisterResult, AnyError> {
+  #[global] function: v8::Global<v8::Function>,
+  #[string] name: String,
+  baseline: bool,
+  #[string] group: Option<String>,
+  ignore: bool,
+  only: bool,
+  warmup: bool,
+  #[buffer] ret_buf: &mut [u8],
+) -> Result<(), AnyError> {
+  if ret_buf.len() != 4 {
+    return Err(type_error(format!(
+      "Invalid ret_buf length: {}",
+      ret_buf.len()
+    )));
+  }
   let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);
   let origin = state.borrow::<ModuleSpecifier>().to_string();
   let description = BenchDescription {
     id,
-    name: info.name,
+    name,
     origin: origin.clone(),
-    baseline: info.baseline,
-    group: info.group,
-    ignore: info.ignore,
-    only: info.only,
-    warmup: info.warmup,
+    baseline,
+    group,
+    ignore,
+    only,
+    warmup,
   };
-  let function: v8::Local<v8::Function> = info.function.v8_value.try_into()?;
-  let function = v8::Global::new(scope, function);
   state
     .borrow_mut::<BenchContainer>()
     .0
     .push((description.clone(), function));
   let sender = state.borrow::<UnboundedSender<BenchEvent>>().clone();
   sender.send(BenchEvent::Register(description)).ok();
-  Ok(BenchRegisterResult { id, origin })
+  ret_buf.copy_from_slice(&(id as u32).to_le_bytes());
+  Ok(())
 }
 
 #[op2]

runtime/js/99_main.js

@@ -564,6 +564,7 @@ const NOT_IMPORTED_OPS = [
   "op_bench_now",
   "op_dispatch_bench_event",
   "op_register_bench",
+  "op_bench_get_origin",
 
   // Related to `Deno.jupyter` API
   "op_jupyter_broadcast",