Undo JsonOpDispatcher and OpDispatcher traits (#7023)

This reverts commit f83d672ffa.
This reverts commit d51972377c.

cli/op_fetch_asset.rs

@@ -4,7 +4,6 @@
 pub use deno_core::v8_set_flags;
 use deno_core::CoreIsolateState;
 use deno_core::Op;
-use deno_core::OpDispatcher;
 use deno_core::ZeroCopyBuf;
 use std::collections::HashMap;
 use std::path::PathBuf;
@@ -88,7 +87,7 @@ fn get_asset(name: &str) -> Option<&'static str> {
 /// CoreIsolate.
 pub fn op_fetch_asset<S: ::std::hash::BuildHasher>(
   custom_assets: HashMap<String, PathBuf, S>,
-) -> impl OpDispatcher {
+) -> impl Fn(&mut deno_core::CoreIsolateState, &mut [ZeroCopyBuf]) -> Op {
   for (_, path) in custom_assets.iter() {
     println!("cargo:rerun-if-changed={}", path.display());
   }

cli/ops/compiler.rs

@@ -2,7 +2,6 @@
 use super::dispatch_json::{JsonOp, Value};
 use crate::op_error::OpError;
 use crate::ops::json_op;
-use crate::ops::JsonOpDispatcher;
 use crate::state::State;
 use deno_core::CoreIsolate;
 use deno_core::CoreIsolateState;
@@ -32,7 +31,11 @@ pub fn init(
 pub fn compiler_op<D>(
   response: Arc<Mutex<Option<String>>>,
   dispatcher: D,
-) -> impl JsonOpDispatcher
+) -> impl Fn(
+  &mut deno_core::CoreIsolateState,
+  Value,
+  &mut [ZeroCopyBuf],
+) -> Result<JsonOp, OpError>
 where
   D: Fn(
     Arc<Mutex<Option<String>>>,

cli/ops/dispatch_json.rs

@@ -3,7 +3,6 @@ use crate::op_error::OpError;
 use deno_core::Buf;
 use deno_core::CoreIsolateState;
 use deno_core::Op;
-use deno_core::OpDispatcher;
 use deno_core::ZeroCopyBuf;
 use futures::future::FutureExt;
 pub use serde_derive::Deserialize;
@@ -45,36 +44,16 @@ struct AsyncArgs {
   promise_id: Option<u64>,
 }
 
-/// Like OpDispatcher but with additional json `Value` parameter
-/// and return a result of `JsonOp` instead of `Op`.
-pub trait JsonOpDispatcher {
-  fn dispatch(
-    &self,
-    isolate_state: &mut CoreIsolateState,
-    json: Value,
-    zero_copy: &mut [ZeroCopyBuf],
-  ) -> Result<JsonOp, OpError>;
-}
-
-impl<F> JsonOpDispatcher for F
+pub fn json_op<D>(
+  d: D,
+) -> impl Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op
 where
-  F: Fn(
+  D: Fn(
     &mut CoreIsolateState,
     Value,
     &mut [ZeroCopyBuf],
   ) -> Result<JsonOp, OpError>,
 {
-  fn dispatch(
-    &self,
-    isolate_state: &mut CoreIsolateState,
-    json: Value,
-    zero_copy: &mut [ZeroCopyBuf],
-  ) -> Result<JsonOp, OpError> {
-    self(isolate_state, json, zero_copy)
-  }
-}
-
-pub fn json_op(d: impl JsonOpDispatcher) -> impl OpDispatcher {
   move |isolate_state: &mut CoreIsolateState, zero_copy: &mut [ZeroCopyBuf]| {
     assert!(!zero_copy.is_empty(), "Expected JSON string at position 0");
     let async_args: AsyncArgs = match serde_json::from_slice(&zero_copy[0]) {
@@ -89,7 +68,7 @@ pub fn json_op(d: impl JsonOpDispatcher) -> impl OpDispatcher {
 
     let result = serde_json::from_slice(&zero_copy[0])
       .map_err(OpError::from)
-      .and_then(|args| d.dispatch(isolate_state, args, &mut zero_copy[1..]));
+      .and_then(|args| d(isolate_state, args, &mut zero_copy[1..]));
 
     // Convert to Op
     match result {

cli/ops/dispatch_minimal.rs

@@ -9,7 +9,6 @@ use byteorder::{LittleEndian, WriteBytesExt};
 use deno_core::Buf;
 use deno_core::CoreIsolateState;
 use deno_core::Op;
-use deno_core::OpDispatcher;
 use deno_core::ZeroCopyBuf;
 use futures::future::FutureExt;
 use std::future::Future;
@@ -119,7 +118,9 @@ fn test_parse_min_record() {
   assert_eq!(parse_min_record(&buf), None);
 }
 
-pub fn minimal_op<D>(d: D) -> impl OpDispatcher
+pub fn minimal_op<D>(
+  d: D,
+) -> impl Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op
 where
   D: Fn(&mut CoreIsolateState, bool, i32, &mut [ZeroCopyBuf]) -> MinimalOp,
 {

cli/ops/mod.rs

@@ -4,7 +4,6 @@ mod dispatch_minimal;
 
 pub use dispatch_json::json_op;
 pub use dispatch_json::JsonOp;
-pub use dispatch_json::JsonOpDispatcher;
 pub use dispatch_json::JsonResult;
 pub use dispatch_minimal::minimal_op;
 pub use dispatch_minimal::MinimalOp;

cli/ops/plugin.rs

@@ -3,6 +3,7 @@ use crate::op_error::OpError;
 use crate::ops::dispatch_json::Deserialize;
 use crate::ops::dispatch_json::JsonOp;
 use crate::ops::dispatch_json::Value;
+use crate::ops::json_op;
 use crate::state::State;
 use deno_core::plugin_api;
 use deno_core::CoreIsolate;
@@ -20,7 +21,10 @@ use std::task::Context;
 use std::task::Poll;
 
 pub fn init(i: &mut CoreIsolate, s: &State) {
-  i.register_op("op_open_plugin", s.stateful_json_op2(op_open_plugin));
+  i.register_op(
+    "op_open_plugin",
+    s.core_op(json_op(s.stateful_op2(op_open_plugin))),
+  );
 }
 
 #[derive(Deserialize)]
@@ -106,8 +110,7 @@ impl<'a> plugin_api::Interface for PluginInterface<'a> {
     let plugin_lib = self.plugin_lib.clone();
     self.isolate_state.op_registry.register(
       name,
-      move |isolate_state: &mut CoreIsolateState,
-            zero_copy: &mut [ZeroCopyBuf]| {
+      move |isolate_state, zero_copy| {
         let mut interface = PluginInterface::new(isolate_state, &plugin_lib);
         let op = dispatch_op_fn(&mut interface, zero_copy);
         match op {

cli/ops/web_worker.rs

@@ -2,7 +2,6 @@
 use super::dispatch_json::{JsonOp, Value};
 use crate::op_error::OpError;
 use crate::ops::json_op;
-use crate::ops::JsonOpDispatcher;
 use crate::state::State;
 use crate::web_worker::WebWorkerHandle;
 use crate::worker::WorkerEvent;
@@ -15,7 +14,11 @@ use std::convert::From;
 pub fn web_worker_op<D>(
   sender: mpsc::Sender<WorkerEvent>,
   dispatcher: D,
-) -> impl JsonOpDispatcher
+) -> impl Fn(
+  &mut CoreIsolateState,
+  Value,
+  &mut [ZeroCopyBuf],
+) -> Result<JsonOp, OpError>
 where
   D: Fn(
     &mpsc::Sender<WorkerEvent>,
@@ -33,7 +36,11 @@ pub fn web_worker_op2<D>(
   handle: WebWorkerHandle,
   sender: mpsc::Sender<WorkerEvent>,
   dispatcher: D,
-) -> impl JsonOpDispatcher
+) -> impl Fn(
+  &mut CoreIsolateState,
+  Value,
+  &mut [ZeroCopyBuf],
+) -> Result<JsonOp, OpError>
 where
   D: Fn(
     WebWorkerHandle,

cli/state.rs

@@ -7,7 +7,6 @@ use crate::import_map::ImportMap;
 use crate::metrics::Metrics;
 use crate::op_error::OpError;
 use crate::ops::JsonOp;
-use crate::ops::JsonOpDispatcher;
 use crate::ops::MinimalOp;
 use crate::permissions::Permissions;
 use crate::tsc::TargetLib;
@@ -18,7 +17,6 @@ use deno_core::ModuleLoadId;
 use deno_core::ModuleLoader;
 use deno_core::ModuleSpecifier;
 use deno_core::Op;
-use deno_core::OpDispatcher;
 use deno_core::ZeroCopyBuf;
 use futures::future::FutureExt;
 use futures::Future;
@@ -66,7 +64,10 @@ pub struct StateInner {
 }
 
 impl State {
-  pub fn stateful_json_op<D>(&self, dispatcher: D) -> impl OpDispatcher
+  pub fn stateful_json_op<D>(
+    &self,
+    dispatcher: D,
+  ) -> impl Fn(&mut deno_core::CoreIsolateState, &mut [ZeroCopyBuf]) -> Op
   where
     D: Fn(&State, Value, &mut [ZeroCopyBuf]) -> Result<JsonOp, OpError>,
   {
@@ -74,7 +75,10 @@ impl State {
     self.core_op(json_op(self.stateful_op(dispatcher)))
   }
 
-  pub fn stateful_json_op2<D>(&self, dispatcher: D) -> impl OpDispatcher
+  pub fn stateful_json_op2<D>(
+    &self,
+    dispatcher: D,
+  ) -> impl Fn(&mut deno_core::CoreIsolateState, &mut [ZeroCopyBuf]) -> Op
   where
     D: Fn(
       &mut deno_core::CoreIsolateState,
@@ -90,7 +94,13 @@ impl State {
   /// Wrap core `OpDispatcher` to collect metrics.
   // TODO(ry) this should be private. Is called by stateful_json_op or
   // stateful_minimal_op
-  pub fn core_op(&self, dispatcher: impl OpDispatcher) -> impl OpDispatcher {
+  pub fn core_op<D>(
+    &self,
+    dispatcher: D,
+  ) -> impl Fn(&mut deno_core::CoreIsolateState, &mut [ZeroCopyBuf]) -> Op
+  where
+    D: Fn(&mut deno_core::CoreIsolateState, &mut [ZeroCopyBuf]) -> Op,
+  {
     let state = self.clone();
 
     move |isolate_state: &mut deno_core::CoreIsolateState,
@@ -101,7 +111,7 @@ impl State {
       let bytes_sent_zero_copy =
         zero_copy[1..].iter().map(|b| b.len()).sum::<usize>() as u64;
 
-      let op = dispatcher.dispatch(isolate_state, zero_copy);
+      let op = dispatcher(isolate_state, zero_copy);
 
       match op {
         Op::Sync(buf) => {
@@ -144,7 +154,10 @@ impl State {
     }
   }
 
-  pub fn stateful_minimal_op2<D>(&self, dispatcher: D) -> impl OpDispatcher
+  pub fn stateful_minimal_op2<D>(
+    &self,
+    dispatcher: D,
+  ) -> impl Fn(&mut deno_core::CoreIsolateState, &mut [ZeroCopyBuf]) -> Op
   where
     D: Fn(
       &mut deno_core::CoreIsolateState,
@@ -171,7 +184,14 @@ impl State {
   /// NOTE: This only works with JSON dispatcher.
   /// This is a band-aid for transition to `CoreIsolate.register_op` API as most of our
   /// ops require `state` argument.
-  pub fn stateful_op<D>(&self, dispatcher: D) -> impl JsonOpDispatcher
+  pub fn stateful_op<D>(
+    &self,
+    dispatcher: D,
+  ) -> impl Fn(
+    &mut deno_core::CoreIsolateState,
+    Value,
+    &mut [ZeroCopyBuf],
+  ) -> Result<JsonOp, OpError>
   where
     D: Fn(&State, Value, &mut [ZeroCopyBuf]) -> Result<JsonOp, OpError>,
   {
@@ -182,7 +202,14 @@ impl State {
           -> Result<JsonOp, OpError> { dispatcher(&state, args, zero_copy) }
   }
 
-  pub fn stateful_op2<D>(&self, dispatcher: D) -> impl JsonOpDispatcher
+  pub fn stateful_op2<D>(
+    &self,
+    dispatcher: D,
+  ) -> impl Fn(
+    &mut deno_core::CoreIsolateState,
+    Value,
+    &mut [ZeroCopyBuf],
+  ) -> Result<JsonOp, OpError>
   where
     D: Fn(
       &mut deno_core::CoreIsolateState,

core/core_isolate.rs

@@ -419,11 +419,10 @@ impl CoreIsolate {
   /// corresponds to the second argument of Deno.core.dispatch().
   ///
   /// Requires runtime to explicitly ask for op ids before using any of the ops.
-  pub fn register_op(
-    &mut self,
-    name: &str,
-    op: impl OpDispatcher + 'static,
-  ) -> OpId {
+  pub fn register_op<F>(&mut self, name: &str, op: F) -> OpId
+  where
+    F: Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op + 'static,
+  {
     let state_rc = Self::state(self);
     let mut state = state_rc.borrow_mut();
     state.op_registry.register(name, op)
@@ -590,7 +589,7 @@ impl CoreIsolateState {
   /// Requires runtime to explicitly ask for op ids before using any of the ops.
   pub fn register_op<F>(&mut self, name: &str, op: F) -> OpId
   where
-    F: OpDispatcher + 'static,
+    F: Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op + 'static,
   {
     self.op_registry.register(name, op)
   }
@@ -612,7 +611,7 @@ impl CoreIsolateState {
     zero_copy_bufs: &mut [ZeroCopyBuf],
   ) -> Option<(OpId, Box<[u8]>)> {
     let op = if let Some(dispatcher) = self.op_registry.get(op_id) {
-      dispatcher.dispatch(self, zero_copy_bufs)
+      dispatcher(self, zero_copy_bufs)
     } else {
       let message =
         v8::String::new(scope, &format!("Unknown op id: {}", op_id)).unwrap();

core/lib.rs

@@ -46,7 +46,6 @@ pub use crate::modules::RecursiveModuleLoad;
 pub use crate::ops::Buf;
 pub use crate::ops::Op;
 pub use crate::ops::OpAsyncFuture;
-pub use crate::ops::OpDispatcher;
 pub use crate::ops::OpId;
 pub use crate::resources::ResourceTable;
 pub use crate::zero_copy_buf::ZeroCopyBuf;

core/ops.rs

@@ -20,53 +20,31 @@ pub enum Op {
   AsyncUnref(OpAsyncFuture),
 }
 
-/// Main type describing Op
-pub trait OpDispatcher {
-  fn dispatch(
-    &self,
-    isolate_state: &mut CoreIsolateState,
-    zero_copy: &mut [ZeroCopyBuf],
-  ) -> Op;
-}
-
-impl<F> OpDispatcher for F
-where
-  F: Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op,
-{
-  fn dispatch(
-    &self,
-    isolate_state: &mut CoreIsolateState,
-    zero_copy: &mut [ZeroCopyBuf],
-  ) -> Op {
-    self(isolate_state, zero_copy)
-  }
-}
+/// Main type describing op
+pub type OpDispatcher =
+  dyn Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op + 'static;
 
 #[derive(Default)]
 pub struct OpRegistry {
-  dispatchers: Vec<Rc<dyn OpDispatcher>>,
+  dispatchers: Vec<Rc<OpDispatcher>>,
   name_to_id: HashMap<String, OpId>,
 }
 
 impl OpRegistry {
   pub fn new() -> Self {
     let mut registry = Self::default();
-    let op_id = registry.register(
-      "ops",
-      |state: &mut CoreIsolateState, _: &mut [ZeroCopyBuf]| {
-        let buf = state.op_registry.json_map();
-        Op::Sync(buf)
-      },
-    );
+    let op_id = registry.register("ops", |state, _| {
+      let buf = state.op_registry.json_map();
+      Op::Sync(buf)
+    });
     assert_eq!(op_id, 0);
     registry
   }
 
-  pub fn register(
-    &mut self,
-    name: &str,
-    op: impl OpDispatcher + 'static,
-  ) -> OpId {
+  pub fn register<F>(&mut self, name: &str, op: F) -> OpId
+  where
+    F: Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op + 'static,
+  {
     let op_id = self.dispatchers.len() as u32;
 
     let existing = self.name_to_id.insert(name.to_string(), op_id);
@@ -83,8 +61,8 @@ impl OpRegistry {
     op_map_json.as_bytes().to_owned().into_boxed_slice()
   }
 
-  pub fn get(&self, op_id: OpId) -> Option<Rc<dyn OpDispatcher>> {
-    self.dispatchers.get(op_id as usize).cloned()
+  pub fn get(&self, op_id: OpId) -> Option<Rc<OpDispatcher>> {
+    self.dispatchers.get(op_id as usize).map(Rc::clone)
   }
 
   pub fn unregister_op(&mut self, name: &str) {
@@ -103,13 +81,10 @@ fn test_op_registry() {
   let c = Arc::new(atomic::AtomicUsize::new(0));
   let c_ = c.clone();
 
-  let test_id = op_registry.register(
-    "test",
-    move |_: &mut CoreIsolateState, _: &mut [ZeroCopyBuf]| {
-      c_.fetch_add(1, atomic::Ordering::SeqCst);
-      Op::Sync(Box::new([]))
-    },
-  );
+  let test_id = op_registry.register("test", move |_, _| {
+    c_.fetch_add(1, atomic::Ordering::SeqCst);
+    Op::Sync(Box::new([]))
+  });
   assert!(test_id != 0);
 
   let mut expected = HashMap::new();
@@ -122,7 +97,7 @@ fn test_op_registry() {
   let dispatch = op_registry.get(test_id).unwrap();
   let state_rc = CoreIsolate::state(&isolate);
   let mut state = state_rc.borrow_mut();
-  let res = dispatch.dispatch(&mut state, &mut []);
+  let res = dispatch(&mut state, &mut []);
   if let Op::Sync(buf) = res {
     assert_eq!(buf.len(), 0);
   } else {
@@ -152,21 +127,15 @@ fn register_op_during_call() {
 
   let test_id = {
     let mut g = op_registry.lock().unwrap();
-    g.register(
-      "dynamic_register_op",
-      move |_: &mut CoreIsolateState, _: &mut [ZeroCopyBuf]| {
-        let c__ = c_.clone();
-        let mut g = op_registry_.lock().unwrap();
-        g.register(
-          "test",
-          move |_: &mut CoreIsolateState, _: &mut [ZeroCopyBuf]| {
-            c__.fetch_add(1, atomic::Ordering::SeqCst);
-            Op::Sync(Box::new([]))
-          },
-        );
+    g.register("dynamic_register_op", move |_, _| {
+      let c__ = c_.clone();
+      let mut g = op_registry_.lock().unwrap();
+      g.register("test", move |_, _| {
+        c__.fetch_add(1, atomic::Ordering::SeqCst);
         Op::Sync(Box::new([]))
-      },
-    )
+      });
+      Op::Sync(Box::new([]))
+    })
   };
   assert!(test_id != 0);
 
@@ -179,7 +148,7 @@ fn register_op_during_call() {
   {
     let state_rc = CoreIsolate::state(&isolate);
     let mut state = state_rc.borrow_mut();
-    dispatcher1.dispatch(&mut state, &mut []);
+    dispatcher1(&mut state, &mut []);
   }
 
   let mut expected = HashMap::new();
@@ -197,7 +166,7 @@ fn register_op_during_call() {
   };
   let state_rc = CoreIsolate::state(&isolate);
   let mut state = state_rc.borrow_mut();
-  let res = dispatcher2.dispatch(&mut state, &mut []);
+  let res = dispatcher2(&mut state, &mut []);
   if let Op::Sync(buf) = res {
     assert_eq!(buf.len(), 0);
   } else {