Skip to content

Commit

Permalink
cleanup(core): replace OpResponse with OpResult (denoland#10434)
Browse files Browse the repository at this point in the history
Drop the Value/Buffer enum since denoland#10432 allows buffers to be serialized rust => v8
  • Loading branch information
AaronO authored Apr 30, 2021
1 parent 5ec478b commit fc9c7de
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 85 deletions.
42 changes: 8 additions & 34 deletions core/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::JsRuntime;
use crate::Op;
use crate::OpId;
use crate::OpPayload;
use crate::OpResponse;
use crate::OpTable;
use crate::PromiseId;
use crate::ZeroCopyBuf;
Expand Down Expand Up @@ -154,19 +153,6 @@ pub fn set_func(
obj.set(scope, key.into(), val.into());
}

pub fn boxed_slice_to_uint8array<'sc>(
scope: &mut v8::HandleScope<'sc>,
buf: Box<[u8]>,
) -> v8::Local<'sc, v8::Uint8Array> {
assert!(!buf.is_empty());
let buf_len = buf.len();
let backing_store = v8::ArrayBuffer::new_backing_store_from_boxed_slice(buf);
let backing_store_shared = backing_store.make_shared();
let ab = v8::ArrayBuffer::with_backing_store(scope, &backing_store_shared);
v8::Uint8Array::new(scope, ab, 0, buf_len)
.expect("Failed to create UintArray8")
}

pub extern "C" fn host_import_module_dynamically_callback(
context: v8::Local<v8::Context>,
referrer: v8::Local<v8::ScriptOrModule>,
Expand Down Expand Up @@ -368,32 +354,20 @@ fn opcall<'s>(

// Buf arg (optional)
let arg3 = args.get(3);
let buf: Option<ZeroCopyBuf> = if arg3.is_null_or_undefined() {
None
} else {
match v8::Local::<v8::ArrayBufferView>::try_from(arg3)
.map(|view| ZeroCopyBuf::new(scope, view))
.map_err(AnyError::from)
{
Ok(buf) => Some(buf),
Err(err) => {
throw_type_error(scope, format!("Err with buf arg: {}", err));
return;
}
let buf: Option<ZeroCopyBuf> = match serde_v8::from_v8(scope, arg3) {
Ok(buf) => buf,
Err(err) => {
throw_type_error(scope, format!("Err with buf arg: {}", err));
return;
}
};

let payload = OpPayload::new(scope, v, promise_id);
let op = OpTable::route_op(op_id, state.op_state.clone(), payload, buf);
match op {
Op::Sync(resp) => match resp {
OpResponse::Value(v) => {
rv.set(v.to_v8(scope).unwrap());
}
OpResponse::Buffer(buf) => {
rv.set(boxed_slice_to_uint8array(scope, buf).into());
}
},
Op::Sync(result) => {
rv.set(result.to_v8(scope).unwrap());
}
Op::Async(fut) => {
state.pending_ops.push(fut);
state.have_unpolled_ops = true;
Expand Down
2 changes: 1 addition & 1 deletion core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub use crate::ops::OpAsyncFuture;
pub use crate::ops::OpFn;
pub use crate::ops::OpId;
pub use crate::ops::OpPayload;
pub use crate::ops::OpResponse;
pub use crate::ops::OpResult;
pub use crate::ops::OpState;
pub use crate::ops::OpTable;
pub use crate::ops::PromiseId;
Expand Down
44 changes: 9 additions & 35 deletions core/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::pin::Pin;
use std::rc::Rc;

pub type PromiseId = u64;
pub type OpAsyncFuture = Pin<Box<dyn Future<Output = (PromiseId, OpResponse)>>>;
pub type OpAsyncFuture = Pin<Box<dyn Future<Output = (PromiseId, OpResult)>>>;
pub type OpFn =
dyn Fn(Rc<RefCell<OpState>>, OpPayload, Option<ZeroCopyBuf>) -> Op + 'static;
pub type OpId = usize;
Expand Down Expand Up @@ -58,13 +58,8 @@ impl<'a, 'b, 'c> OpPayload<'a, 'b, 'c> {
}
}

pub enum OpResponse {
Value(OpResult),
Buffer(Box<[u8]>),
}

pub enum Op {
Sync(OpResponse),
Sync(OpResult),
Async(OpAsyncFuture),
/// AsyncUnref is the variation of Async, which doesn't block the program
/// exiting.
Expand Down Expand Up @@ -100,14 +95,14 @@ pub struct OpError {
pub fn serialize_op_result<R: Serialize + 'static>(
result: Result<R, AnyError>,
state: Rc<RefCell<OpState>>,
) -> OpResponse {
OpResponse::Value(match result {
) -> OpResult {
match result {
Ok(v) => OpResult::Ok(v.into()),
Err(err) => OpResult::Err(OpError {
class_name: (state.borrow().get_error_class_fn)(&err),
message: err.to_string(),
}),
})
}
}

/// Maintains the resources and ops inside a JS runtime.
Expand Down Expand Up @@ -205,35 +200,14 @@ mod tests {
let bar_id;
{
let op_table = &mut state.borrow_mut().op_table;
foo_id = op_table.register_op("foo", |_, _, _| {
Op::Sync(OpResponse::Buffer(b"oof!"[..].into()))
});
foo_id = op_table
.register_op("foo", |_, _, _| Op::Sync(OpResult::Ok(321.into())));
assert_eq!(foo_id, 1);
bar_id = op_table.register_op("bar", |_, _, _| {
Op::Sync(OpResponse::Buffer(b"rab!"[..].into()))
});
bar_id = op_table
.register_op("bar", |_, _, _| Op::Sync(OpResult::Ok(123.into())));
assert_eq!(bar_id, 2);
}

let foo_res = OpTable::route_op(
foo_id,
state.clone(),
OpPayload::empty(),
Default::default(),
);
assert!(
matches!(foo_res, Op::Sync(OpResponse::Buffer(buf)) if &*buf == b"oof!")
);
let bar_res = OpTable::route_op(
bar_id,
state.clone(),
OpPayload::empty(),
Default::default(),
);
assert!(
matches!(bar_res, Op::Sync(OpResponse::Buffer(buf)) if &*buf == b"rab!")
);

let mut catalog_entries = OpTable::op_entries(state);
catalog_entries.sort_by(|(_, id1), (_, id2)| id1.partial_cmp(id2).unwrap());
assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion core/plugin_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

pub use crate::Op;
pub use crate::OpId;
pub use crate::OpResponse;
pub use crate::OpResult;
pub use crate::ZeroCopyBuf;

pub type InitFn = fn(&mut dyn Interface);
Expand Down
17 changes: 6 additions & 11 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::ops::*;
use crate::Extension;
use crate::OpMiddlewareFn;
use crate::OpPayload;
use crate::OpResponse;
use crate::OpResult;
use crate::OpState;
use crate::PromiseId;
use crate::ZeroCopyBuf;
Expand All @@ -49,7 +49,7 @@ use std::sync::Once;
use std::task::Context;
use std::task::Poll;

type PendingOpFuture = Pin<Box<dyn Future<Output = (PromiseId, OpResponse)>>>;
type PendingOpFuture = Pin<Box<dyn Future<Output = (PromiseId, OpResult)>>>;

pub enum Snapshot {
Static(&'static [u8]),
Expand Down Expand Up @@ -1394,9 +1394,9 @@ impl JsRuntime {
fn poll_pending_ops(
&mut self,
cx: &mut Context,
) -> Vec<(PromiseId, OpResponse)> {
) -> Vec<(PromiseId, OpResult)> {
let state_rc = Self::state(self.v8_isolate());
let mut async_responses: Vec<(PromiseId, OpResponse)> = Vec::new();
let mut async_responses: Vec<(PromiseId, OpResult)> = Vec::new();

let mut state = state_rc.borrow_mut();

Expand Down Expand Up @@ -1455,7 +1455,7 @@ impl JsRuntime {
// Send finished responses to JS
fn async_op_response(
&mut self,
async_responses: Vec<(PromiseId, OpResponse)>,
async_responses: Vec<(PromiseId, OpResult)>,
) -> Result<(), AnyError> {
let state_rc = Self::state(self.v8_isolate());

Expand All @@ -1480,12 +1480,7 @@ impl JsRuntime {
for overflown_response in async_responses {
let (promise_id, resp) = overflown_response;
args.push(v8::Integer::new(scope, promise_id as i32).into());
args.push(match resp {
OpResponse::Value(value) => value.to_v8(scope).unwrap(),
OpResponse::Buffer(buf) => {
bindings::boxed_slice_to_uint8array(scope, buf).into()
}
});
args.push(resp.to_v8(scope).unwrap());
}

let tc_scope = &mut v8::TryCatch::new(scope);
Expand Down
6 changes: 3 additions & 3 deletions test_plugin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use deno_core::plugin_api::Interface;
use deno_core::plugin_api::Op;
use deno_core::plugin_api::OpResponse;
use deno_core::plugin_api::OpResult;
use deno_core::plugin_api::ZeroCopyBuf;
use futures::future::FutureExt;

Expand All @@ -25,7 +25,7 @@ fn op_test_sync(
}
let result = b"test";
let result_box: Box<[u8]> = Box::new(*result);
Op::Sync(OpResponse::Buffer(result_box))
Op::Sync(OpResult::Ok(result_box.into()))
}

fn op_test_async(
Expand All @@ -48,7 +48,7 @@ fn op_test_async(
assert!(rx.await.is_ok());
let result = b"test";
let result_box: Box<[u8]> = Box::new(*result);
(0, OpResponse::Buffer(result_box))
(0, OpResult::Ok(result_box.into()))
};

Op::Async(fut.boxed())
Expand Down

0 comments on commit fc9c7de

Please sign in to comment.