diff --git a/cli/op_fetch_asset.rs b/cli/op_fetch_asset.rs index d89ae20b39c77..0ebd84c4308a2 100644 --- a/cli/op_fetch_asset.rs +++ b/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( custom_assets: HashMap, -) -> impl OpDispatcher { +) -> impl Fn(&mut deno_core::CoreIsolateState, &mut [ZeroCopyBuf]) -> Op { for (_, path) in custom_assets.iter() { println!("cargo:rerun-if-changed={}", path.display()); } diff --git a/cli/ops/compiler.rs b/cli/ops/compiler.rs index 5af2f5eb05571..d41369855370c 100644 --- a/cli/ops/compiler.rs +++ b/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( response: Arc>>, dispatcher: D, -) -> impl JsonOpDispatcher +) -> impl Fn( + &mut deno_core::CoreIsolateState, + Value, + &mut [ZeroCopyBuf], +) -> Result where D: Fn( Arc>>, diff --git a/cli/ops/dispatch_json.rs b/cli/ops/dispatch_json.rs index ad6947dd1f006..2ec9d6c2f0807 100644 --- a/cli/ops/dispatch_json.rs +++ b/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, } -/// 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; -} - -impl JsonOpDispatcher for F +pub fn json_op( + d: D, +) -> impl Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op where - F: Fn( + D: Fn( &mut CoreIsolateState, Value, &mut [ZeroCopyBuf], ) -> Result, { - fn dispatch( - &self, - isolate_state: &mut CoreIsolateState, - json: Value, - zero_copy: &mut [ZeroCopyBuf], - ) -> Result { - 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 { diff --git a/cli/ops/dispatch_minimal.rs b/cli/ops/dispatch_minimal.rs index 9cb81d4bce10f..df7d48b4e4efd 100644 --- a/cli/ops/dispatch_minimal.rs +++ b/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) -> impl OpDispatcher +pub fn minimal_op( + d: D, +) -> impl Fn(&mut CoreIsolateState, &mut [ZeroCopyBuf]) -> Op where D: Fn(&mut CoreIsolateState, bool, i32, &mut [ZeroCopyBuf]) -> MinimalOp, { diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs index 331ed4aa1969f..ef8c3bd0f16b7 100644 --- a/cli/ops/mod.rs +++ b/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; diff --git a/cli/ops/plugin.rs b/cli/ops/plugin.rs index 3edcaa962f561..16debac50a2b7 100644 --- a/cli/ops/plugin.rs +++ b/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 { diff --git a/cli/ops/web_worker.rs b/cli/ops/web_worker.rs index 4a661d2bedaf9..553278b0741e1 100644 --- a/cli/ops/web_worker.rs +++ b/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( sender: mpsc::Sender, dispatcher: D, -) -> impl JsonOpDispatcher +) -> impl Fn( + &mut CoreIsolateState, + Value, + &mut [ZeroCopyBuf], +) -> Result where D: Fn( &mpsc::Sender, @@ -33,7 +36,11 @@ pub fn web_worker_op2( handle: WebWorkerHandle, sender: mpsc::Sender, dispatcher: D, -) -> impl JsonOpDispatcher +) -> impl Fn( + &mut CoreIsolateState, + Value, + &mut [ZeroCopyBuf], +) -> Result where D: Fn( WebWorkerHandle, diff --git a/cli/state.rs b/cli/state.rs index f04b577b42d8b..35aaa7ed2ccdd 100644 --- a/cli/state.rs +++ b/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(&self, dispatcher: D) -> impl OpDispatcher + pub fn stateful_json_op( + &self, + dispatcher: D, + ) -> impl Fn(&mut deno_core::CoreIsolateState, &mut [ZeroCopyBuf]) -> Op where D: Fn(&State, Value, &mut [ZeroCopyBuf]) -> Result, { @@ -74,7 +75,10 @@ impl State { self.core_op(json_op(self.stateful_op(dispatcher))) } - pub fn stateful_json_op2(&self, dispatcher: D) -> impl OpDispatcher + pub fn stateful_json_op2( + &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( + &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::() 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(&self, dispatcher: D) -> impl OpDispatcher + pub fn stateful_minimal_op2( + &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(&self, dispatcher: D) -> impl JsonOpDispatcher + pub fn stateful_op( + &self, + dispatcher: D, + ) -> impl Fn( + &mut deno_core::CoreIsolateState, + Value, + &mut [ZeroCopyBuf], + ) -> Result where D: Fn(&State, Value, &mut [ZeroCopyBuf]) -> Result, { @@ -182,7 +202,14 @@ impl State { -> Result { dispatcher(&state, args, zero_copy) } } - pub fn stateful_op2(&self, dispatcher: D) -> impl JsonOpDispatcher + pub fn stateful_op2( + &self, + dispatcher: D, + ) -> impl Fn( + &mut deno_core::CoreIsolateState, + Value, + &mut [ZeroCopyBuf], + ) -> Result where D: Fn( &mut deno_core::CoreIsolateState, diff --git a/core/core_isolate.rs b/core/core_isolate.rs index 17345e14c7cfe..199bbd832a93b 100644 --- a/core/core_isolate.rs +++ b/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(&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(&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(); diff --git a/core/lib.rs b/core/lib.rs index f348ed6cf468e..7358af1c43456 100644 --- a/core/lib.rs +++ b/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; diff --git a/core/ops.rs b/core/ops.rs index 603526dbcbcc8..65a0f325b136f 100644 --- a/core/ops.rs +++ b/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 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>, + dispatchers: Vec>, name_to_id: HashMap, } 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(&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> { - self.dispatchers.get(op_id as usize).cloned() + pub fn get(&self, op_id: OpId) -> Option> { + 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 {