Merge pull request #815 from neon-bindings/kv/easy-perf-wins

perf: Performance improvements for Node-API backend

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<Local> {
+        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)
 }

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) }
     }
 
@@ -579,7 +577,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,10 +702,13 @@ 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<SmallVec<[raw::Local; 8]>>,
+    arguments: Option<neon_runtime::call::Arguments>,
     phantom_type: PhantomData<T>,
 }
 
@@ -757,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)))
         }
     }
 
@@ -835,7 +837,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")]