From f23a3a0b965189e913db2c0ad497ed3510210ce1 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Thu, 28 Oct 2021 10:22:56 -0400 Subject: [PATCH 1/2] perf(neon): Switch to `InheritedHandleScope` on most `Context` structs Node-API creates handle scopes as part of the environment. Creating an explicit scope is only necessary for `Context::compute_scoped` and `Context::execute_scoped`. --- src/context/mod.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/context/mod.rs b/src/context/mod.rs index e0fdb9313..4ed8c46f8 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -579,7 +579,10 @@ pub trait Context<'a>: ContextInternal<'a> { /// An execution context of module initialization. pub struct ModuleContext<'a> { + #[cfg(feature = "legacy-runtime")] scope: Scope<'a, raw::HandleScope>, + #[cfg(feature = "napi-1")] + scope: Scope<'a, raw::InheritedHandleScope>, exports: Handle<'a, JsObject>, } @@ -701,7 +704,10 @@ impl<'a, 'b> Context<'a> for ComputeContext<'a, 'b> {} /// /// The type parameter `T` is the type of the `this`-binding. pub struct CallContext<'a, T: This> { + #[cfg(feature = "legacy-runtime")] scope: Scope<'a, raw::HandleScope>, + #[cfg(feature = "napi-1")] + scope: Scope<'a, raw::InheritedHandleScope>, info: &'a CallbackInfo<'a>, #[cfg(feature = "napi-1")] arguments: Option>, @@ -835,7 +841,7 @@ impl<'a> Context<'a> for TaskContext<'a> {} /// A view of the JS engine in the context of a finalize method on garbage collection #[cfg(feature = "napi-1")] pub(crate) struct FinalizeContext<'a> { - scope: Scope<'a, raw::HandleScope>, + scope: Scope<'a, raw::InheritedHandleScope>, } #[cfg(feature = "napi-1")] From a417371bad1af020339cef4ad5bfbbdaa108406f Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Thu, 28 Oct 2021 11:07:09 -0400 Subject: [PATCH 2/2] perf(neon-runtime): Try to only call `napi_cb_info` once Additionally, avoids initialization of allocated space --- crates/neon-runtime/src/napi/call.rs | 98 +++++++++++++++++++++++----- src/context/mod.rs | 20 +++--- 2 files changed, 91 insertions(+), 27 deletions(-) diff --git a/crates/neon-runtime/src/napi/call.rs b/crates/neon-runtime/src/napi/call.rs index a8c917d53..4075fb54b 100644 --- a/crates/neon-runtime/src/napi/call.rs +++ b/crates/neon-runtime/src/napi/call.rs @@ -2,11 +2,46 @@ use std::mem::MaybeUninit; use std::os::raw::c_void; use std::ptr::null_mut; -use smallvec::{smallvec, SmallVec}; +use smallvec::SmallVec; use crate::napi::bindings as napi; use crate::raw::{Env, FunctionCallbackInfo, Local}; +// Number of arguments to allocate on the stack. This should be large enough +// to cover most use cases without being wasteful. +// +// * If the number is too large, too much space is allocated and then filled +// with `undefined`. +// * If the number is too small, getting arguments frequently takes two tries +// and requires heap allocation. +const ARGV_SIZE: usize = 4; + +#[repr(transparent)] +/// List of JavaScript arguments to a function +// `Arguments` is intended to be a small abstraction to hide the usage of +// `SmallVec` allowing changes to `ARGV_SIZE` in a single location +pub struct Arguments(SmallVec<[Local; ARGV_SIZE]>); + +impl Arguments { + #[inline] + /// Get an argument at a specific position + pub fn get(&self, i: usize) -> Option { + self.0.get(i).cloned() + } + + #[inline] + /// Returns the number of arguments + pub fn len(&self) -> usize { + self.0.len() + } + + #[inline] + /// `true` if there are no arguments + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + #[repr(C)] pub struct CCallback { pub static_callback: *mut c_void, @@ -61,6 +96,7 @@ pub unsafe fn data(env: Env, info: FunctionCallbackInfo, out: &mut *mut c_void) } /// Gets the number of arguments passed to the function. +// TODO: Remove this when `CallContext` is refactored to get call info upfront. pub unsafe fn len(env: Env, info: FunctionCallbackInfo) -> i32 { let mut argc = 0usize; let status = napi::get_cb_info( @@ -75,19 +111,51 @@ pub unsafe fn len(env: Env, info: FunctionCallbackInfo) -> i32 { argc as i32 } -/// Returns the function arguments as a `SmallVec<[Local; 8]>` -pub unsafe fn argv(env: Env, info: FunctionCallbackInfo) -> SmallVec<[Local; 8]> { - let len = len(env, info); - let mut args = smallvec![null_mut(); len as usize]; - let mut num_args = args.len(); - let status = napi::get_cb_info( - env, - info, - &mut num_args as *mut _, - args.as_mut_ptr(), - null_mut(), - null_mut(), +/// Returns the function arguments for a call +pub unsafe fn argv(env: Env, info: FunctionCallbackInfo) -> Arguments { + // Allocate space on the stack for up to `ARGV_SIZE` values + let mut argv = MaybeUninit::<[Local; ARGV_SIZE]>::uninit(); + + // Starts as the size allocated; after `get_cb_info` it is the number of arguments + let mut argc = ARGV_SIZE; + + assert_eq!( + napi::get_cb_info( + env, + info, + &mut argc as *mut _, + argv.as_mut_ptr().cast(), + null_mut(), + null_mut(), + ), + napi::Status::Ok, ); - assert_eq!(status, napi::Status::Ok); - args + + // We did not allocate enough space; allocate on the heap and try again + let argv = if argc > ARGV_SIZE { + // We know exactly how much space to reserve + let mut argv = Vec::with_capacity(argc); + + assert_eq!( + napi::get_cb_info( + env, + info, + &mut argc as *mut _, + argv.as_mut_ptr(), + null_mut(), + null_mut(), + ), + napi::Status::Ok, + ); + + // Set the size of `argv` to the number of initialized elements + argv.set_len(argc); + SmallVec::from_vec(argv) + + // There were `ARGV_SIZE` or fewer arguments, use the stack allocated space + } else { + SmallVec::from_buf_and_len(argv.assume_init(), argc) + }; + + Arguments(argv) } diff --git a/src/context/mod.rs b/src/context/mod.rs index 4ed8c46f8..39c99da1b 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -172,8 +172,6 @@ use crate::types::{ }; use neon_runtime; use neon_runtime::raw; -#[cfg(feature = "napi-1")] -use smallvec::SmallVec; use std; use std::cell::RefCell; use std::convert::Into; @@ -246,7 +244,7 @@ impl CallbackInfo<'_> { } #[cfg(feature = "napi-1")] - pub fn argv<'b, C: Context<'b>>(&self, cx: &mut C) -> SmallVec<[raw::Local; 8]> { + pub fn argv<'b, C: Context<'b>>(&self, cx: &mut C) -> neon_runtime::call::Arguments { unsafe { neon_runtime::call::argv(cx.env().to_raw(), self.info) } } @@ -710,7 +708,7 @@ pub struct CallContext<'a, T: This> { scope: Scope<'a, raw::InheritedHandleScope>, info: &'a CallbackInfo<'a>, #[cfg(feature = "napi-1")] - arguments: Option>, + arguments: Option, phantom_type: PhantomData, } @@ -763,17 +761,15 @@ impl<'a, T: This> CallContext<'a, T> { #[cfg(feature = "napi-1")] { - let local = if let Some(arguments) = &self.arguments { - arguments.get(i as usize).cloned() + let argv = if let Some(argv) = self.arguments.as_ref() { + argv } else { - let arguments = self.info.argv(self); - let local = arguments.get(i as usize).cloned(); - - self.arguments = Some(arguments); - local + let argv = self.info.argv(self); + self.arguments.insert(argv) }; - local.map(|local| Handle::new_internal(JsValue::from_raw(self.env(), local))) + argv.get(i as usize) + .map(|v| Handle::new_internal(JsValue::from_raw(self.env(), v))) } }