Skip to content

Commit

Permalink
Undo JsonOpDispatcher and OpDispatcher traits (denoland#7023)
Browse files Browse the repository at this point in the history
This reverts commit f83d672.
This reverts commit d519723.
  • Loading branch information
ry committed Aug 12, 2020
1 parent 6706eb5 commit f5a4f1f
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 116 deletions.
3 changes: 1 addition & 2 deletions cli/op_fetch_asset.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
7 changes: 5 additions & 2 deletions cli/ops/compiler.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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>>>,
Expand Down
31 changes: 5 additions & 26 deletions cli/ops/dispatch_json.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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]) {
Expand All @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions cli/ops/dispatch_minimal.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
{
Expand Down
1 change: 0 additions & 1 deletion cli/ops/mod.rs
Expand Up @@ -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;
Expand Down
9 changes: 6 additions & 3 deletions cli/ops/plugin.rs
Expand Up @@ -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;
Expand All @@ -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)]
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 10 additions & 3 deletions cli/ops/web_worker.rs
Expand Up @@ -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;
Expand All @@ -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>,
Expand All @@ -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,
Expand Down
45 changes: 36 additions & 9 deletions cli/state.rs
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -66,15 +64,21 @@ 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>,
{
use crate::ops::json_op;
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,
Expand All @@ -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,
Expand All @@ -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) => {
Expand Down Expand Up @@ -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,
Expand All @@ -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>,
{
Expand All @@ -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,
Expand Down
13 changes: 6 additions & 7 deletions core/core_isolate.rs
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand All @@ -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();
Expand Down
1 change: 0 additions & 1 deletion core/lib.rs
Expand Up @@ -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;
Expand Down

0 comments on commit f5a4f1f

Please sign in to comment.