Skip to content

Commit

Permalink
Revert "feat(ops): V8 Fast Calls (denoland#15122)" (denoland#15276)
Browse files Browse the repository at this point in the history
This reverts commit 03dc3b8.
  • Loading branch information
littledivy committed Jul 22, 2022
1 parent 03dc3b8 commit 4db650d
Show file tree
Hide file tree
Showing 17 changed files with 32 additions and 494 deletions.
23 changes: 0 additions & 23 deletions Cargo.lock

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

25 changes: 1 addition & 24 deletions cli/bench/deno_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,8 @@ 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.call(a, b);
}
// deno-lint-ignore no-inner-declarations
function addJS(a, b) {
return a + b;
}
Deno.bench("op_add", { n: 1e7 }, () => add(1, 2));
Deno.bench("add_js", { n: 1e7 }, () => addJS(1, 2));
}

// deno-lint-ignore camelcase
const { op_void_sync } = Deno.core.ops;
function sync() {
return op_void_sync.call();
}
sync(); // Warmup
console.log(sync());
// Void ops measure op-overhead
Deno.bench("op_void_sync", { n: 1e7 }, () => sync());
Deno.bench("op_void_sync", { n: 1e7 }, () => Deno.core.opSync("op_void_sync"));

Deno.bench(
"op_void_async",
Expand Down
4 changes: 2 additions & 2 deletions core/01_core.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@

function opAsync(opName, ...args) {
const promiseId = nextPromiseId++;
const maybeError = ops[opName].call(promiseId, ...args);
const maybeError = ops[opName](promiseId, ...args);
// Handle sync error (e.g: error parsing args)
if (maybeError) return unwrapOpResult(maybeError);
let p = PromisePrototypeThen(setPromise(promiseId), unwrapOpResult);
Expand All @@ -167,7 +167,7 @@
}

function opSync(opName, ...args) {
return unwrapOpResult(ops[opName].call(...args));
return unwrapOpResult(ops[opName](...args));
}

function refOp(promiseId) {
Expand Down
80 changes: 13 additions & 67 deletions core/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,17 @@ 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 fn external_references(ops: &[OpCtx]) -> v8::ExternalReferences {
let mut references = vec![
v8::ExternalReference {
pub static EXTERNAL_REFERENCES: Lazy<v8::ExternalReferences> =
Lazy::new(|| {
v8::ExternalReferences::new(&[v8::ExternalReference {
function: call_console.map_fn_to(),
},
v8::ExternalReference {
pointer: ops as *const _ as _,
},
];

for ctx in ops {
let ctx_ptr = ctx as *const OpCtx as _;
references.push(v8::ExternalReference { pointer: ctx_ptr });
references.push(v8::ExternalReference {
function: ctx.decl.v8_fn_ptr,
});
if let Some(fast_fn) = &ctx.decl.fast_fn {
references.push(v8::ExternalReference {
pointer: fast_fn.function() as _,
});
}
}

v8::ExternalReferences::new(&references)
}
}])
});

// TODO(nayeemrmn): Move to runtime and/or make `pub(crate)`.
pub fn script_origin<'a>(
Expand Down Expand Up @@ -101,8 +82,7 @@ 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, snapshot_loaded);

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

Expand All @@ -114,41 +94,18 @@ 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, snapshot_loaded);
initialize_ops(scope, ops_obj, op_ctxs);
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;

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,
"call",
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());
set_func_raw(scope, ops_obj, ctx.decl.name, ctx.decl.v8_fn_ptr, ctx_ptr);
}
}

Expand All @@ -172,24 +129,13 @@ 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 builder =
v8::FunctionTemplate::builder_raw(callback).data(external.into());
let templ = if let Some(fast_function) = fast_function {
if !snapshot_loaded {
builder.build(scope)
} else {
// TODO(@littledivy): Support fast api overloads in ops.
builder.build_fast(scope, &**fast_function, None)
}
} else {
builder.build(scope)
};
let val = templ.get_function(scope).unwrap();
let val = v8::Function::builder_raw(callback)
.data(external.into())
.build(scope)
.unwrap();
val.set_name(key);
obj.set(scope, key.into(), val.into());
}
Expand Down
9 changes: 3 additions & 6 deletions core/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,21 @@
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;

pub trait FastFunctionSignature {}

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

impl OpDecl {
Expand All @@ -35,7 +32,7 @@ impl OpDecl {
#[derive(Default)]
pub struct Extension {
js_files: Option<Vec<SourcePair>>,
pub ops: Option<Vec<OpDecl>>,
ops: Option<Vec<OpDecl>>,
opstate_fn: Option<Box<OpStateFn>>,
middleware_fn: Option<Box<OpMiddlewareFn>>,
event_loop_middleware: Option<Box<OpEventLoopFn>>,
Expand Down
3 changes: 0 additions & 3 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,9 @@ pub fn v8_version() -> &'static str {
pub mod _ops {
pub use super::bindings::throw_type_error;
pub use super::error_codes::get_error_code;
pub use super::extensions::FastFunctionSignature;
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: 1 addition & 7 deletions core/ops_builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ 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 @@ -55,12 +54,7 @@ pub fn op_resources(state: &mut OpState) -> Vec<(ResourceId, String)> {
.collect()
}

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

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

#[op]
Expand Down
19 changes: 5 additions & 14 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@ fn v8_init(
" --harmony-import-assertions",
" --no-validate-asm",
" --turbo_fast_api_calls",
" --allow-natives-syntax",
);

if predictable {
Expand All @@ -243,9 +242,6 @@ fn v8_init(
}
}

pub const V8_WRAPPER_TYPE_INDEX: i32 = 0;
pub const V8_WRAPPER_OBJECT_INDEX: i32 = 1;

#[derive(Default)]
pub struct RuntimeOptions {
/// Source map reference for errors.
Expand Down Expand Up @@ -321,6 +317,7 @@ 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 @@ -333,13 +330,12 @@ impl JsRuntime {
.collect::<Vec<_>>()
.into_boxed_slice();

let refs = bindings::external_references(&op_ctxs);
let refs: &'static v8::ExternalReferences = Box::leak(Box::new(refs));
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(refs));
let mut creator =
v8::SnapshotCreator::new(Some(&bindings::EXTERNAL_REFERENCES));
// 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 @@ -356,13 +352,8 @@ impl JsRuntime {
let mut params = options
.create_params
.take()
.unwrap_or_else(|| {
v8::Isolate::create_params().embedder_wrapper_type_info_offsets(
V8_WRAPPER_TYPE_INDEX,
V8_WRAPPER_OBJECT_INDEX,
)
})
.external_references(&**refs);
.unwrap_or_else(v8::Isolate::create_params)
.external_references(&**bindings::EXTERNAL_REFERENCES);
let snapshot_loaded = if let Some(snapshot) = options.startup_snapshot {
params = match snapshot {
Snapshot::Static(data) => params.snapshot_blob(data),
Expand Down
4 changes: 1 addition & 3 deletions ext/net/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,14 @@ 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)
.ops([&ops::init::<P>()[..], &ops_tls::init::<P>()[..]].concat())
.state(move |state| {
state.put(DefaultTlsOptions {
root_cert_store: root_cert_store.clone(),
Expand Down
4 changes: 1 addition & 3 deletions ext/web/02_timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@
const { webidl } = window.__bootstrap;
const { reportException } = window.__bootstrap.event;
const { assert } = window.__bootstrap.infra;
// deno-lint-ignore camelcase
const { op_now } = Deno.core.ops;

function opNow() {
return op_now.call();
return core.opSync("op_now");
}

// ---------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion ext/web/timers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub type StartTime = Instant;
// If the High precision flag is not set, the
// nanoseconds are rounded on 2ms.
#[op]
pub fn op_now<TP>(state: &mut OpState) -> f64
pub fn op_now<TP>(state: &mut OpState, _argument: ()) -> f64
where
TP: TimersPermission + 'static,
{
Expand Down
Loading

0 comments on commit 4db650d

Please sign in to comment.