fix(core): op metrics op_names mismatch (denoland#14716)

cli/tests/unit/metrics_test.ts

@@ -1,5 +1,5 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
-import { assert } from "./test_util.ts";
+import { assert, assertEquals } from "./test_util.ts";
 
 Deno.test(async function metrics() {
  // Write to stdout to ensure a "data" message gets sent instead of just
@@ -76,3 +76,14 @@ Deno.test(function metricsForOpCrates() {
  assert(m1.opsDispatched > 0);
  assert(m1.opsCompleted > 0);
 });
+
+// Test that op_names == Objects.keys(Deno.core.ops)
+// since building the per-op metrics depends on op_names being complete
+Deno.test(function opNamesMatch() {
+ assertEquals(
+ // @ts-ignore: Deno.core allowed
+ Deno.core.opNames().sort(),
+ // @ts-ignore: Deno.core allowed
+ Object.keys(Deno.core.ops).sort(),
+ );
+});

core/01_core.js

@@ -217,7 +217,7 @@
  function metrics() {
  const [aggregate, perOps] = opSync("op_metrics");
  aggregate.ops = ObjectFromEntries(ArrayPrototypeMap(
- core.op_names,
+ core.opNames(),
  (opName, opId) => [opName, perOps[opId]],
  ));
  return aggregate;

core/bindings.rs

@@ -114,6 +114,9 @@ pub static EXTERNAL_REFERENCES: Lazy<v8::ExternalReferences> =
  v8::ExternalReference {
  function: apply_source_map.map_fn_to(),
  },
+ v8::ExternalReference {
+ function: op_names.map_fn_to(),
+ },
  ])
  });
 
@@ -174,13 +177,10 @@ pub fn initialize_context<'s>(
  // a really weird usecase. Remove this once all
  // tsc ops are static at snapshot time.
  if snapshot_loaded {
- // Grab the Deno.core & Deno.core.ops objects
- let core_obj = JsRuntime::grab_global::<v8::Object>(scope, "Deno.core")
- .expect("Deno.core to exist");
+ // Grab the Deno.core.ops object & init it
  let ops_obj = JsRuntime::grab_global::<v8::Object>(scope, "Deno.core.ops")
  .expect("Deno.core.ops to exist");
  initialize_ops(scope, ops_obj, op_ctxs);
- initialize_op_names(scope, core_obj, op_ctxs);
  return scope.escape(context);
  }
 
@@ -243,14 +243,14 @@ pub fn initialize_context<'s>(
  set_func(scope, core_val, "destructureError", destructure_error);
  set_func(scope, core_val, "terminate", terminate);
  set_func(scope, core_val, "applySourceMap", apply_source_map);
+ set_func(scope, core_val, "opNames", op_names);
 
  // Direct bindings on `window`.
  set_func(scope, global, "queueMicrotask", queue_microtask);
 
  // Bind functions to Deno.core.ops.*
  let ops_obj = JsRuntime::ensure_objs(scope, global, "Deno.core.ops").unwrap();
  initialize_ops(scope, ops_obj, op_ctxs);
- initialize_op_names(scope, core_val, op_ctxs);
  scope.escape(context)
 }
 
@@ -265,17 +265,6 @@ fn initialize_ops(
  }
 }
 
-fn initialize_op_names(
- scope: &mut v8::HandleScope,
- core_obj: v8::Local<v8::Object>,
- op_ctxs: &[OpCtx],
-) {
- let names: Vec<&str> = op_ctxs.iter().map(|o| o.decl.name).collect();
- let k = v8::String::new(scope, "op_names").unwrap().into();
- let v = serde_v8::to_v8(scope, names).unwrap();
- core_obj.set(scope, k, v);
-}
-
 pub fn set_func(
  scope: &mut v8::HandleScope<'_>,
  obj: v8::Local<v8::Object>,
@@ -1553,3 +1542,14 @@ fn get_memory_usage(isolate: &mut v8::Isolate) -> MemoryUsage {
  external: s.external_memory(),
  }
 }
+
+fn op_names(
+ scope: &mut v8::HandleScope,
+ _args: v8::FunctionCallbackArguments,
+ mut rv: v8::ReturnValue,
+) {
+ let state_rc = JsRuntime::state(scope);
+ let state = state_rc.borrow();
+ let names: Vec<_> = state.op_ctxs.iter().map(|o| o.decl.name).collect();
+ rv.set(serde_v8::to_v8(scope, names).unwrap());
+}