From 57b1101a9374a66df5cca0632ae78b2fa82b5976 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Fri, 12 Nov 2021 13:40:30 -0500 Subject: [PATCH] Simplify `call` and `construct` to only take a `AsRef>` that can be directly passed to args A future PR will add a high level builder API for calling functions. In order to maintain a low level, high performance API, the existing APIs have been adapted to not require unnecessary allocation. --- crates/neon-runtime/src/napi/fun.rs | 4 +-- crates/neon-sys/src/lib.rs | 4 +-- src/event/event_handler.rs | 6 ++++- src/object/class/mod.rs | 5 ++++ src/types/mod.rs | 33 +++++++++++-------------- test/dynamic/native/src/js/functions.rs | 4 +-- test/napi/src/js/functions.rs | 4 +-- test/napi/src/js/threads.rs | 10 ++++---- 8 files changed, 37 insertions(+), 33 deletions(-) diff --git a/crates/neon-runtime/src/napi/fun.rs b/crates/neon-runtime/src/napi/fun.rs index f05ab51b5..297cfb7e3 100644 --- a/crates/neon-runtime/src/napi/fun.rs +++ b/crates/neon-runtime/src/napi/fun.rs @@ -32,7 +32,7 @@ pub unsafe fn call( fun: Local, this: Local, argc: i32, - argv: *mut c_void, + argv: *const c_void, ) -> bool { let status = napi::call_function( env, @@ -51,7 +51,7 @@ pub unsafe fn construct( env: Env, fun: Local, argc: i32, - argv: *mut c_void, + argv: *const c_void, ) -> bool { let status = napi::new_instance(env, fun, argc as usize, argv as *const _, out as *mut _); diff --git a/crates/neon-sys/src/lib.rs b/crates/neon-sys/src/lib.rs index d7910b618..149a83787 100644 --- a/crates/neon-sys/src/lib.rs +++ b/crates/neon-sys/src/lib.rs @@ -206,14 +206,14 @@ extern "C" { fun: Local, this: Local, argc: i32, - argv: *mut c_void, + argv: *const c_void, ) -> bool; pub fn Neon_Fun_Construct( out: &mut Local, isolate: Isolate, fun: Local, argc: i32, - argv: *mut c_void, + argv: *const c_void, ) -> bool; pub fn Neon_Mem_SameHandle(h1: Local, h2: Local) -> bool; diff --git a/src/event/event_handler.rs b/src/event/event_handler.rs index cb9e3b120..5fe68a946 100644 --- a/src/event/event_handler.rs +++ b/src/event/event_handler.rs @@ -47,7 +47,11 @@ impl EventHandler { F: Send + 'static, { self.schedule_with(move |cx, this, callback| { - let args = arg_cb(cx); + let args = arg_cb(cx) + .into_iter() + .map(|v| v.upcast()) + .collect::>(); + let _result = callback.call(cx, this, args); }) } diff --git a/src/object/class/mod.rs b/src/object/class/mod.rs index dff27a59e..8b5c50791 100644 --- a/src/object/class/mod.rs +++ b/src/object/class/mod.rs @@ -109,6 +109,11 @@ pub trait Class: Managed + Any { AS: IntoIterator>, { let constructor = Self::constructor(cx)?; + let args = args + .into_iter() + .map(|v| v.upcast()) + .collect::>(); + constructor.construct(cx, args) } diff --git a/src/types/mod.rs b/src/types/mod.rs index 348fb8028..c7365740f 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -100,7 +100,6 @@ use crate::result::{JsResult, JsResultExt, NeonResult, Throw}; use crate::types::internal::Callback; use neon_runtime; use neon_runtime::raw; -use smallvec::SmallVec; use std::fmt; use std::fmt::Debug; use std::marker::PhantomData; @@ -694,19 +693,19 @@ impl Object for JsFunction {} // Maximum number of function arguments in V8. const V8_ARGC_LIMIT: usize = 65535; -unsafe fn prepare_call<'a, 'b, C: Context<'a>, A>( +unsafe fn prepare_call<'a, 'b, C: Context<'a>>( cx: &mut C, - args: &mut [Handle<'b, A>], -) -> NeonResult<(i32, *mut c_void)> -where - A: Value + 'b, -{ - let argv = args.as_mut_ptr(); + args: &[Handle<'b, JsValue>], +) -> NeonResult<(i32, *const c_void)> { + // Note: This cast is only save because `Handle<'_, JsValue>` is + // guaranteed to have the same layout as a pointer because `Handle` + // and `JsValue` are both `repr(C)` newtypes. + let argv = args.as_ptr().cast(); let argc = args.len(); if argc > V8_ARGC_LIMIT { return cx.throw_range_error("too many arguments"); } - Ok((argc as i32, argv as *mut c_void)) + Ok((argc as i32, argv)) } impl JsFunction { @@ -729,7 +728,7 @@ impl JsFunction { } impl JsFunction { - pub fn call<'a, 'b, C: Context<'a>, T, A, AS>( + pub fn call<'a, 'b, C: Context<'a>, T, AS>( self, cx: &mut C, this: Handle<'b, T>, @@ -737,24 +736,20 @@ impl JsFunction { ) -> JsResult<'a, JsValue> where T: Value, - A: Value + 'b, - AS: IntoIterator>, + AS: AsRef<[Handle<'b, JsValue>]>, { - let mut args = args.into_iter().collect::>(); - let (argc, argv) = unsafe { prepare_call(cx, &mut args) }?; + let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?; let env = cx.env().to_raw(); build(cx.env(), |out| unsafe { neon_runtime::fun::call(out, env, self.to_raw(), this.to_raw(), argc, argv) }) } - pub fn construct<'a, 'b, C: Context<'a>, A, AS>(self, cx: &mut C, args: AS) -> JsResult<'a, CL> + pub fn construct<'a, 'b, C: Context<'a>, AS>(self, cx: &mut C, args: AS) -> JsResult<'a, CL> where - A: Value + 'b, - AS: IntoIterator>, + AS: AsRef<[Handle<'b, JsValue>]>, { - let mut args = args.into_iter().collect::>(); - let (argc, argv) = unsafe { prepare_call(cx, &mut args) }?; + let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?; let env = cx.env().to_raw(); build(cx.env(), |out| unsafe { neon_runtime::fun::construct(out, env, self.to_raw(), argc, argv) diff --git a/test/dynamic/native/src/js/functions.rs b/test/dynamic/native/src/js/functions.rs index cafc80715..687391e87 100644 --- a/test/dynamic/native/src/js/functions.rs +++ b/test/dynamic/native/src/js/functions.rs @@ -13,7 +13,7 @@ pub fn return_js_function(mut cx: FunctionContext) -> JsResult { pub fn call_js_function(mut cx: FunctionContext) -> JsResult { let f = cx.argument::(0)?; - let args: Vec> = vec![cx.number(16.0)]; + let args = [cx.number(16.0).upcast()]; let null = cx.null(); f.call(&mut cx, null, args)? .downcast::() @@ -23,7 +23,7 @@ pub fn call_js_function(mut cx: FunctionContext) -> JsResult { pub fn construct_js_function(mut cx: FunctionContext) -> JsResult { let f = cx.argument::(0)?; let zero = cx.number(0.0); - let o = f.construct(&mut cx, vec![zero])?; + let o = f.construct(&mut cx, [zero.upcast()])?; let get_utc_full_year_method = o .get(&mut cx, "getUTCFullYear")? .downcast::() diff --git a/test/napi/src/js/functions.rs b/test/napi/src/js/functions.rs index 5a8bd819c..2a8cbcaf3 100644 --- a/test/napi/src/js/functions.rs +++ b/test/napi/src/js/functions.rs @@ -12,7 +12,7 @@ pub fn return_js_function(mut cx: FunctionContext) -> JsResult { pub fn call_js_function(mut cx: FunctionContext) -> JsResult { let f = cx.argument::(0)?; - let args: Vec> = vec![cx.number(16.0)]; + let args = [cx.number(16.0).upcast()]; let null = cx.null(); f.call(&mut cx, null, args)? .downcast::(&mut cx) @@ -22,7 +22,7 @@ pub fn call_js_function(mut cx: FunctionContext) -> JsResult { pub fn construct_js_function(mut cx: FunctionContext) -> JsResult { let f = cx.argument::(0)?; let zero = cx.number(0.0); - let o = f.construct(&mut cx, vec![zero])?; + let o = f.construct(&mut cx, [zero.upcast()])?; let get_utc_full_year_method = o .get(&mut cx, "getUTCFullYear")? .downcast::(&mut cx) diff --git a/test/napi/src/js/threads.rs b/test/napi/src/js/threads.rs index 9e3de5702..e8048d5b2 100644 --- a/test/napi/src/js/threads.rs +++ b/test/napi/src/js/threads.rs @@ -45,7 +45,7 @@ pub fn multi_threaded_callback(mut cx: FunctionContext) -> JsResult channel.send(move |mut cx| { let callback = callback.into_inner(&mut cx); let this = cx.undefined(); - let args = vec![cx.number(i as f64)]; + let args = [cx.number(i as f64).upcast()]; callback.call(&mut cx, this, args)?; @@ -78,7 +78,7 @@ impl AsyncGreeter { channel.send(|mut cx| { let callback = callback.into_inner(&mut cx); let this = cx.undefined(); - let args = vec![cx.string(greeting)]; + let args = [cx.string(greeting).upcast()]; callback.call(&mut cx, this, args)?; @@ -163,7 +163,7 @@ pub fn drop_global_queue(mut cx: FunctionContext) -> JsResult { self.channel.send(|mut cx| { let callback = callback.into_inner(&mut cx); let this = cx.undefined(); - let args = vec![cx.undefined()]; + let args = [cx.undefined().upcast()]; callback.call(&mut cx, this, args)?; @@ -207,7 +207,7 @@ pub fn channel_join(mut cx: FunctionContext) -> JsResult { get_message .into_inner(&mut cx) - .call::<_, _, JsValue, _>(&mut cx, this, [])? + .call(&mut cx, this, [])? .downcast_or_throw::(&mut cx) .map(|v| v.value(&mut cx)) }) @@ -220,7 +220,7 @@ pub fn channel_join(mut cx: FunctionContext) -> JsResult { // Call back to JavaScript with the response channel.send(move |mut cx| { let this = cx.undefined(); - let args = [cx.string(response)]; + let args = [cx.string(response).upcast()]; callback.into_inner(&mut cx).call(&mut cx, this, args)?;