Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ops): V8 Fast Calls #15291

Merged
merged 39 commits into from
Aug 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a4892f7
feat(ops): V8 Fast Calls
littledivy Jul 8, 2022
3f89ca3
oink
littledivy Jul 8, 2022
204f360
works for sync ops
littledivy Jul 8, 2022
94a8a02
hmm
littledivy Jul 9, 2022
bf4e11e
Merge branch 'main' of github.com:denoland/deno into perf/fast-calls
littledivy Jul 9, 2022
fc89ad2
snapshots and work
littledivy Jul 9, 2022
f0ff99d
x
littledivy Jul 10, 2022
125a5c2
Merge branch 'main' of github.com:denoland/deno into perf/fast-calls
littledivy Jul 16, 2022
c20825b
:rocket:
littledivy Jul 17, 2022
6295af1
some work
littledivy Jul 17, 2022
fb91815
debug
littledivy Jul 17, 2022
537eac9
generics
littledivy Jul 20, 2022
78bbeb3
Merge branch 'main' of github.com:denoland/deno into perf/fast-calls
littledivy Jul 20, 2022
4380f7c
fixes
littledivy Jul 20, 2022
4313d89
x
littledivy Jul 20, 2022
efcc641
r
littledivy Jul 20, 2022
cfa0687
format
littledivy Jul 20, 2022
c5cc7e8
revert some test stuff
littledivy Jul 20, 2022
278905e
Merge branch 'main' into perf/fast-calls
bartlomieju Jul 21, 2022
41a701f
update std submodule
bartlomieju Jul 21, 2022
b00b7d1
copyright and example
bartlomieju Jul 21, 2022
be7ca32
lint
bartlomieju Jul 21, 2022
6745546
update readme
littledivy Jul 22, 2022
f08358a
add tests for op macro
bartlomieju Jul 22, 2022
a35028b
Merge branch 'main' of github.com:denoland/deno into perf/fast-calls
littledivy Jul 23, 2022
66734b7
Merge branch 'main' into perf/fast-calls
littledivy Jul 28, 2022
96f5a59
maybe fixed?
littledivy Aug 5, 2022
2463178
ci
littledivy Aug 5, 2022
ea9b1fb
ci
littledivy Aug 5, 2022
7ff6211
ci
littledivy Aug 5, 2022
6ad9eb0
ci
littledivy Aug 5, 2022
cf5d0ba
Merge branch 'main' into perf/fast-calls
littledivy Aug 5, 2022
35b5fa9
h
littledivy Aug 5, 2022
a75ab10
Merge branch 'perf/fast-calls' of github.com:littledivy/deno into per…
littledivy Aug 5, 2022
709ce5d
Merge branch 'main' of github.com:denoland/deno into perf/fast-calls
littledivy Aug 20, 2022
8db574b
what
littledivy Aug 20, 2022
87e4036
Merge branch 'main' into perf/fast-calls
littledivy Aug 20, 2022
781816a
review
littledivy Aug 21, 2022
5c38093
Merge branch 'main' into perf/fast-calls
littledivy Aug 21, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 24 additions & 5 deletions cli/bench/deno_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,31 @@ Deno.bench("date_now", { n: 5e5 }, () => {
Date.now();
});

// Fast API calls
{
// deno-lint-ignore camelcase
const { op_add } = Deno.core.ops;
// deno-lint-ignore no-inner-declarations
function add(a, b) {
return op_add.fast(a, b);
}
// deno-lint-ignore no-inner-declarations
function addJS(a, b) {
return a + b;
}
Deno.bench("op_add", () => add(1, 2));
Deno.bench("add_js", () => addJS(1, 2));
}

// deno-lint-ignore camelcase
const { op_void_sync } = Deno.core.ops;
function sync() {
return op_void_sync.fast();
}
sync(); // Warmup

// Void ops measure op-overhead
Deno.bench(
"op_void_sync",
{ n: 1e7 },
() => Deno.core.ops.op_void_sync(),
);
Deno.bench("op_void_sync", () => sync());

Deno.bench(
"op_void_async",
Expand Down
100 changes: 86 additions & 14 deletions core/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,39 @@ use crate::modules::ModuleMap;
use crate::ops::OpCtx;
use crate::JsRuntime;
use log::debug;
use once_cell::sync::Lazy;
use std::option::Option;
use std::os::raw::c_void;
use v8::fast_api::FastFunction;
use v8::MapFnTo;

pub static EXTERNAL_REFERENCES: Lazy<v8::ExternalReferences> =
Lazy::new(|| {
v8::ExternalReferences::new(&[v8::ExternalReference {
function: call_console.map_fn_to(),
}])
});
pub fn external_references(
ops: &[OpCtx],
snapshot_loaded: bool,
) -> v8::ExternalReferences {
let mut references = vec![v8::ExternalReference {
function: call_console.map_fn_to(),
}];

for ctx in ops {
let ctx_ptr = ctx as *const OpCtx as _;
references.push(v8::ExternalReference { pointer: ctx_ptr });
references.push(v8::ExternalReference {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are all this external refs now suddenly needed when previously they were not?

function: ctx.decl.v8_fn_ptr,
});
if snapshot_loaded {
if let Some(fast_fn) = &ctx.decl.fast_fn {
references.push(v8::ExternalReference {
pointer: fast_fn.function() as _,
});
}
}
}

let refs = v8::ExternalReferences::new(&references);
// Leak, V8 takes ownership of the references.
std::mem::forget(references);
refs
}

// TODO(nayeemrmn): Move to runtime and/or make `pub(crate)`.
pub fn script_origin<'a>(
Expand Down Expand Up @@ -82,7 +104,8 @@ pub fn initialize_context<'s>(
// Grab the Deno.core.ops object & init it
let ops_obj = JsRuntime::grab_global::<v8::Object>(scope, "Deno.core.ops")
.expect("Deno.core.ops to exist");
initialize_ops(scope, ops_obj, op_ctxs);
initialize_ops(scope, ops_obj, op_ctxs, snapshot_loaded);

return scope.escape(context);
}

Expand All @@ -94,18 +117,55 @@ pub fn initialize_context<'s>(

// Bind functions to Deno.core.ops.*
let ops_obj = JsRuntime::ensure_objs(scope, global, "Deno.core.ops").unwrap();
initialize_ops(scope, ops_obj, op_ctxs);

initialize_ops(scope, ops_obj, op_ctxs, snapshot_loaded);
scope.escape(context)
}

fn initialize_ops(
scope: &mut v8::HandleScope,
ops_obj: v8::Local<v8::Object>,
op_ctxs: &[OpCtx],
snapshot_loaded: bool,
) {
for ctx in op_ctxs {
let ctx_ptr = ctx as *const OpCtx as *const c_void;
set_func_raw(scope, ops_obj, ctx.decl.name, ctx.decl.v8_fn_ptr, ctx_ptr);

// If this is a fast op, we don't want it to be in the snapshot.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this explains why the boolean is used here, it does not actually explain why the fast op is not wanted in the snapshot. At least I would like to understand why the fast op is left out: Is it because of the doubling of the OpCtx and slow call function pointer referencing?

// Only initialize once snapshot is loaded.
if ctx.decl.fast_fn.is_some() && snapshot_loaded {
littledivy marked this conversation as resolved.
Show resolved Hide resolved
let object_template = v8::ObjectTemplate::new(scope);
assert!(object_template.set_internal_field_count(
(crate::runtime::V8_WRAPPER_OBJECT_INDEX + 1) as usize
));

let method_obj = object_template.new_instance(scope).unwrap();
method_obj.set_aligned_pointer_in_internal_field(
crate::runtime::V8_WRAPPER_OBJECT_INDEX,
ctx_ptr,
);
set_func_raw(
scope,
method_obj,
"fast",
ctx.decl.v8_fn_ptr,
ctx_ptr,
&ctx.decl.fast_fn,
snapshot_loaded,
);
let method_key = v8::String::new(scope, ctx.decl.name).unwrap();
ops_obj.set(scope, method_key.into(), method_obj.into());
} else {
set_func_raw(
scope,
ops_obj,
ctx.decl.name,
ctx.decl.v8_fn_ptr,
ctx_ptr,
&None,
snapshot_loaded,
);
}
}
}

Expand All @@ -129,13 +189,25 @@ pub fn set_func_raw(
name: &'static str,
callback: v8::FunctionCallback,
external_data: *const c_void,
fast_function: &Option<Box<dyn FastFunction>>,
snapshot_loaded: bool,
) {
let key = v8::String::new(scope, name).unwrap();
let external = v8::External::new(scope, external_data as *mut c_void);
let val = v8::Function::builder_raw(callback)
.data(external.into())
.build(scope)
.unwrap();
let builder =
v8::FunctionTemplate::builder_raw(callback).data(external.into());
let templ = if let Some(fast_function) = fast_function {
// Don't initialize fast ops when snapshotting, the external references count mismatch.
if !snapshot_loaded {
builder.build(scope)
} else {
// TODO(@littledivy): Support fast api overloads in ops.
builder.build_fast(scope, &**fast_function, None)
}
littledivy marked this conversation as resolved.
Show resolved Hide resolved
} else {
builder.build(scope)
};
let val = templ.get_function(scope).unwrap();
val.set_name(key);
obj.set(scope, key.into(), val.into());
}
Expand Down
5 changes: 3 additions & 2 deletions core/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,22 @@
use crate::OpState;
use anyhow::Error;
use std::{cell::RefCell, rc::Rc, task::Context};
use v8::fast_api::FastFunction;

pub type SourcePair = (&'static str, &'static str);
pub type OpFnRef = v8::FunctionCallback;
pub type OpMiddlewareFn = dyn Fn(OpDecl) -> OpDecl;
pub type OpStateFn = dyn Fn(&mut OpState) -> Result<(), Error>;
pub type OpEventLoopFn = dyn Fn(Rc<RefCell<OpState>>, &mut Context) -> bool;

#[derive(Clone, Copy)]
pub struct OpDecl {
pub name: &'static str,
pub v8_fn_ptr: OpFnRef,
pub enabled: bool,
pub is_async: bool, // TODO(@AaronO): enum sync/async/fast ?
pub is_async: bool,
pub is_unstable: bool,
pub is_v8: bool,
pub fast_fn: Option<Box<dyn FastFunction>>,
}

impl OpDecl {
Expand Down
2 changes: 2 additions & 0 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ pub mod _ops {
pub use super::ops::to_op_result;
pub use super::ops::OpCtx;
pub use super::runtime::queue_async_op;
pub use super::runtime::V8_WRAPPER_OBJECT_INDEX;
pub use super::runtime::V8_WRAPPER_TYPE_INDEX;
}

/// A helper macro that will return a call site in Rust code. Should be
Expand Down
8 changes: 7 additions & 1 deletion core/ops_builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub(crate) fn init_builtins() -> Extension {
op_wasm_streaming_set_url::decl(),
op_void_sync::decl(),
op_void_async::decl(),
op_add::decl(),
// // TODO(@AaronO): track IO metrics for builtin streams
op_read::decl(),
op_write::decl(),
Expand All @@ -54,7 +55,12 @@ pub fn op_resources(state: &mut OpState) -> Vec<(ResourceId, String)> {
.collect()
}

#[op]
#[op(fast)]
fn op_add(a: i32, b: i32) -> i32 {
a + b
}

#[op(fast)]
pub fn op_void_sync() {}

#[op]
Expand Down
9 changes: 5 additions & 4 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ impl JsRuntime {
if let Some(get_error_class_fn) = options.get_error_class_fn {
op_state.get_error_class_fn = get_error_class_fn;
}

let op_state = Rc::new(RefCell::new(op_state));
let op_ctxs = ops
.into_iter()
Expand All @@ -341,12 +340,14 @@ impl JsRuntime {
.collect::<Vec<_>>()
.into_boxed_slice();

let refs = bindings::external_references(&op_ctxs, !options.will_snapshot);
// V8 takes ownership of external_references.
let refs: &'static v8::ExternalReferences = Box::leak(Box::new(refs));
littledivy marked this conversation as resolved.
Show resolved Hide resolved
let global_context;
let (mut isolate, maybe_snapshot_creator) = if options.will_snapshot {
// TODO(ry) Support loading snapshots before snapshotting.
assert!(options.startup_snapshot.is_none());
let mut creator =
v8::SnapshotCreator::new(Some(&bindings::EXTERNAL_REFERENCES));
let mut creator = v8::SnapshotCreator::new(Some(refs));
// SAFETY: `get_owned_isolate` is unsafe because it may only be called
// once. This is the only place we call this function, so this call is
// safe.
Expand All @@ -369,7 +370,7 @@ impl JsRuntime {
V8_WRAPPER_OBJECT_INDEX,
)
})
.external_references(&**bindings::EXTERNAL_REFERENCES);
.external_references(&**refs);
let snapshot_loaded = if let Some(snapshot) = options.startup_snapshot {
params = match snapshot {
Snapshot::Static(data) => params.snapshot_blob(data),
Expand Down
8 changes: 4 additions & 4 deletions ext/flash/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1344,17 +1344,17 @@ async fn op_flash_next_async(
// the ContextScope creation is optimized away and the op is as simple as:
// f(info: *const v8::FunctionCallbackInfo) { let rv = ...; rv.set_uint32(op_flash_next()); }
#[op]
fn op_flash_next(op_state: &mut OpState) -> u32 {
let flash_ctx = op_state.borrow_mut::<FlashContext>();
fn op_flash_next(state: &mut OpState) -> u32 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated changes?

let flash_ctx = state.borrow_mut::<FlashContext>();
let ctx = flash_ctx.servers.get_mut(&0).unwrap();
next_request_sync(ctx)
}

// Syncrhonous version of op_flash_next_async. Under heavy load,
// this can collect buffered requests from rx channel and return tokens in a single batch.
#[op]
fn op_flash_next_server(op_state: &mut OpState, server_id: u32) -> u32 {
let flash_ctx = op_state.borrow_mut::<FlashContext>();
fn op_flash_next_server(state: &mut OpState, server_id: u32) -> u32 {
let flash_ctx = state.borrow_mut::<FlashContext>();
let ctx = flash_ctx.servers.get_mut(&server_id).unwrap();
next_request_sync(ctx)
}
Expand Down
4 changes: 3 additions & 1 deletion ext/net/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,16 @@ pub fn init<P: NetPermissions + 'static>(
unstable: bool,
unsafely_ignore_certificate_errors: Option<Vec<String>>,
) -> Extension {
let mut ops = ops::init::<P>();
ops.extend(ops_tls::init::<P>());
Extension::builder()
.js(include_js_files!(
prefix "deno:ext/net",
"01_net.js",
"02_tls.js",
"04_net_unstable.js",
))
.ops([&ops::init::<P>()[..], &ops_tls::init::<P>()[..]].concat())
.ops(ops)
.state(move |state| {
state.put(DefaultTlsOptions {
root_cert_store: root_cert_store.clone(),
Expand Down
4 changes: 4 additions & 0 deletions ops/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@ proc-macro2 = "1"
quote = "1"
regex = "1.6.0"
syn = { version = "1", features = ["full", "extra-traits"] }

[dev-dependencies]
deno_core = { path = "../core" }
trybuild = "1.0.61"
Loading