Skip to content

Commit

Permalink
perf(core/ops): avoid allocs when returning primitives (denoland#10149)
Browse files Browse the repository at this point in the history
  • Loading branch information
AaronO committed Apr 12, 2021
1 parent 73b7bd9 commit 2eafbf2
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 18 deletions.
7 changes: 4 additions & 3 deletions core/benches/op_baseline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ use bencher::{benchmark_group, benchmark_main, Bencher};
use deno_core::error::AnyError;
use deno_core::op_async;
use deno_core::op_sync;
use deno_core::serialize_op_result;
use deno_core::v8;
use deno_core::JsRuntime;
use deno_core::Op;
use deno_core::OpResponse;
use deno_core::OpState;
use deno_core::ZeroCopyBuf;

Expand All @@ -17,8 +17,9 @@ fn create_js_runtime() -> JsRuntime {
let mut runtime = JsRuntime::new(Default::default());
runtime.register_op("pi_json", op_sync(|_, _: (), _| Ok(314159)));
runtime.register_op("pi_async", op_async(op_pi_async));
runtime
.register_op("nop", |_, _, _| Op::Sync(OpResponse::Value(Box::new(9))));
runtime.register_op("nop", |state, _, _| {
Op::Sync(serialize_op_result(Ok(9), state))
});

// Init ops
runtime
Expand Down
1 change: 1 addition & 0 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ mod bindings;
pub mod error;
mod flags;
mod gotham_state;
mod minvalue;
mod module_specifier;
mod modules;
mod normalize_path;
Expand Down
95 changes: 95 additions & 0 deletions core/minvalue.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
use rusty_v8 as v8;
use std::any::TypeId;

/// SerializablePkg exists to provide a fast path for op returns,
/// allowing them to avoid boxing primtives (ints/floats/bool/unit/...)
pub enum SerializablePkg {
MinValue(MinValue),
Serializable(Box<dyn serde_v8::Serializable>),
}

impl SerializablePkg {
pub fn to_v8<'a>(
&self,
scope: &mut v8::HandleScope<'a>,
) -> Result<v8::Local<'a, v8::Value>, serde_v8::Error> {
match &*self {
Self::MinValue(x) => serde_v8::to_v8(scope, x),
Self::Serializable(x) => x.to_v8(scope),
}
}
}

/// MinValue serves as a lightweight serializable wrapper around primitives
/// so that we can use them for async values
#[derive(Clone, Copy)]
pub enum MinValue {
Unit(()),
Bool(bool),
Int8(i8),
Int16(i16),
Int32(i32),
Int64(i64),
UInt8(u8),
UInt16(u16),
UInt32(u32),
UInt64(u64),
Float32(f32),
Float64(f64),
}

impl serde::Serialize for MinValue {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
match *self {
Self::Unit(_) => serializer.serialize_unit(),
Self::Bool(x) => serializer.serialize_bool(x),
Self::Int8(x) => serializer.serialize_i8(x),
Self::Int16(x) => serializer.serialize_i16(x),
Self::Int32(x) => serializer.serialize_i32(x),
Self::Int64(x) => serializer.serialize_i64(x),
Self::UInt8(x) => serializer.serialize_u8(x),
Self::UInt16(x) => serializer.serialize_u16(x),
Self::UInt32(x) => serializer.serialize_u32(x),
Self::UInt64(x) => serializer.serialize_u64(x),
Self::Float32(x) => serializer.serialize_f32(x),
Self::Float64(x) => serializer.serialize_f64(x),
}
}
}

impl<T: serde::Serialize + 'static> From<T> for SerializablePkg {
fn from(x: T) -> Self {
let tid = TypeId::of::<T>();

if tid == TypeId::of::<()>() {
Self::MinValue(MinValue::Unit(()))
} else if tid == TypeId::of::<bool>() {
Self::MinValue(MinValue::Bool(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<i8>() {
Self::MinValue(MinValue::Int8(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<i16>() {
Self::MinValue(MinValue::Int16(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<i32>() {
Self::MinValue(MinValue::Int32(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<i64>() {
Self::MinValue(MinValue::Int64(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<u8>() {
Self::MinValue(MinValue::UInt8(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<u16>() {
Self::MinValue(MinValue::UInt16(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<u32>() {
Self::MinValue(MinValue::UInt32(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<u64>() {
Self::MinValue(MinValue::UInt64(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<f32>() {
Self::MinValue(MinValue::Float32(unsafe { std::mem::transmute_copy(&x) }))
} else if tid == TypeId::of::<f64>() {
Self::MinValue(MinValue::Float64(unsafe { std::mem::transmute_copy(&x) }))
} else {
Self::Serializable(Box::new(x))
}
}
}
27 changes: 19 additions & 8 deletions core/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::error::bad_resource_id;
use crate::error::type_error;
use crate::error::AnyError;
use crate::gotham_state::GothamState;
use crate::minvalue::SerializablePkg;
use crate::resources::ResourceId;
use crate::resources::ResourceTable;
use crate::runtime::GetErrorClassFn;
Expand Down Expand Up @@ -61,7 +62,7 @@ impl<'a, 'b, 'c> OpPayload<'a, 'b, 'c> {
}

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

Expand All @@ -74,13 +75,23 @@ pub enum Op {
NotFound,
}

#[derive(Serialize)]
#[serde(untagged)]
pub enum OpResult<R> {
Ok(R),
pub enum OpResult {
Ok(SerializablePkg),
Err(OpError),
}

impl OpResult {
pub fn to_v8<'a>(
&self,
scope: &mut v8::HandleScope<'a>,
) -> Result<v8::Local<'a, v8::Value>, serde_v8::Error> {
match self {
Self::Ok(x) => x.to_v8(scope),
Self::Err(err) => serde_v8::to_v8(scope, err),
}
}
}

#[derive(Serialize)]
#[serde(rename_all = "camelCase")]
pub struct OpError {
Expand All @@ -93,13 +104,13 @@ pub fn serialize_op_result<R: Serialize + 'static>(
result: Result<R, AnyError>,
state: Rc<RefCell<OpState>>,
) -> OpResponse {
OpResponse::Value(Box::new(match result {
Ok(v) => OpResult::Ok(v),
OpResponse::Value(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
15 changes: 8 additions & 7 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1501,18 +1501,19 @@ pub mod tests {
}

fn dispatch(
op_state: Rc<RefCell<OpState>>,
rc_op_state: Rc<RefCell<OpState>>,
payload: OpPayload,
buf: Option<ZeroCopyBuf>,
) -> Op {
let op_state_ = op_state.borrow();
let rc_op_state2 = rc_op_state.clone();
let op_state_ = rc_op_state2.borrow();
let test_state = op_state_.borrow::<TestState>();
test_state.dispatch_count.fetch_add(1, Ordering::Relaxed);
match test_state.mode {
Mode::Async => {
let control: u8 = payload.deserialize().unwrap();
assert_eq!(control, 42);
let resp = (0, OpResponse::Value(Box::new(43)));
let resp = (0, serialize_op_result(Ok(43), rc_op_state));
Op::Async(Box::pin(futures::future::ready(resp)))
}
Mode::AsyncUnref => {
Expand All @@ -1521,7 +1522,7 @@ pub mod tests {
let fut = async {
// This future never finish.
futures::future::pending::<()>().await;
(0, OpResponse::Value(Box::new(43)))
(0, serialize_op_result(Ok(43), rc_op_state))
};
Op::AsyncUnref(Box::pin(fut))
}
Expand All @@ -1531,7 +1532,7 @@ pub mod tests {
assert_eq!(buf.len(), 1);
}

let resp = OpResponse::Value(Box::new(43));
let resp = serialize_op_result(Ok(43), rc_op_state);
Op::Async(Box::pin(futures::future::ready((0, resp))))
}
}
Expand Down Expand Up @@ -1972,11 +1973,11 @@ pub mod tests {
let dispatch_count = Arc::new(AtomicUsize::new(0));
let dispatch_count_ = dispatch_count.clone();

let dispatcher = move |_state, payload: OpPayload, _buf| -> Op {
let dispatcher = move |state, payload: OpPayload, _buf| -> Op {
dispatch_count_.fetch_add(1, Ordering::Relaxed);
let control: u8 = payload.deserialize().unwrap();
assert_eq!(control, 42);
let resp = (0, OpResponse::Value(Box::new(43)));
let resp = (0, serialize_op_result(Ok(43), state));
Op::Async(Box::pin(futures::future::ready(resp)))
};

Expand Down

0 comments on commit 2eafbf2

Please sign in to comment.