From b81dec97024dc8144105b6651d10b63e8536defc Mon Sep 17 00:00:00 2001 From: Alexander Azarov Date: Sat, 25 Sep 2021 12:31:57 +0300 Subject: [PATCH 1/5] Fix broken link in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 08206e18f..324aec168 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Rust bindings for writing safe and fast native Node.js modules. ## Getting started -Once you have the [platform dependencies](https://neon-bindings.com/docs/getting-started#install-node-build-tools/) installed, getting started is as simple as: +Once you have the [platform dependencies](https://neon-bindings.com/docs/quick-start) installed, getting started is as simple as: ``` $ npm init neon my-project From 675f640a416852fdcf309409cc3f838ce7b7b569 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Oct 2021 22:29:36 +0000 Subject: [PATCH 2/5] Bump electron from 11.1.0 to 11.5.0 in /test/electron Bumps [electron](https://github.com/electron/electron) from 11.1.0 to 11.5.0. - [Release notes](https://github.com/electron/electron/releases) - [Changelog](https://github.com/electron/electron/blob/main/docs/breaking-changes.md) - [Commits](https://github.com/electron/electron/compare/v11.1.0...v11.5.0) --- updated-dependencies: - dependency-name: electron dependency-type: direct:development ... Signed-off-by: dependabot[bot] --- test/electron/package-lock.json | 14 +++++++------- test/electron/package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/electron/package-lock.json b/test/electron/package-lock.json index f1d8c6016..b26938890 100644 --- a/test/electron/package-lock.json +++ b/test/electron/package-lock.json @@ -11,7 +11,7 @@ "license": "MIT", "devDependencies": { "cargo-cp-artifact": "^0.1.0 ", - "electron": "^11.1.0", + "electron": "^11.5.0", "spectron": "^13.0.0" } }, @@ -995,9 +995,9 @@ "dev": true }, "node_modules/electron": { - "version": "11.1.0", - "resolved": "https://registry.npmjs.org/electron/-/electron-11.1.0.tgz", - "integrity": "sha512-RFAhR/852VMaRd9NSe7jprwSoG9dLc6u1GwnqRWg+/3cy/8Zrwt1Betw1lXiZH7hGuB9K2cqju83Xv5Pq5ZSGA==", + "version": "11.5.0", + "resolved": "https://registry.npmjs.org/electron/-/electron-11.5.0.tgz", + "integrity": "sha512-WjNDd6lGpxyiNjE3LhnFCAk/D9GIj1rU3GSDealVShhkkkPR3Vh4q8ErXGDl1OAO/faomVa10KoFPUN/pLbNxg==", "dev": true, "hasInstallScript": true, "dependencies": { @@ -3830,9 +3830,9 @@ "dev": true }, "electron": { - "version": "11.1.0", - "resolved": "https://registry.npmjs.org/electron/-/electron-11.1.0.tgz", - "integrity": "sha512-RFAhR/852VMaRd9NSe7jprwSoG9dLc6u1GwnqRWg+/3cy/8Zrwt1Betw1lXiZH7hGuB9K2cqju83Xv5Pq5ZSGA==", + "version": "11.5.0", + "resolved": "https://registry.npmjs.org/electron/-/electron-11.5.0.tgz", + "integrity": "sha512-WjNDd6lGpxyiNjE3LhnFCAk/D9GIj1rU3GSDealVShhkkkPR3Vh4q8ErXGDl1OAO/faomVa10KoFPUN/pLbNxg==", "dev": true, "requires": { "@electron/get": "^1.0.1", diff --git a/test/electron/package.json b/test/electron/package.json index eabe59baa..365e17bb7 100644 --- a/test/electron/package.json +++ b/test/electron/package.json @@ -13,7 +13,7 @@ "repository": "https://github.com/electron/electron-quick-start", "devDependencies": { "cargo-cp-artifact": "^0.1.0 ", - "electron": "^11.1.0", + "electron": "^11.5.0", "spectron": "^13.0.0" } } From e6a9683f9814e1888d1a24b27ccb97a89f5cdef2 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 20 Oct 2021 10:17:35 -0700 Subject: [PATCH 3/5] Use syn-mid instead of syn + "full" Neon's use of syn is for the #[neon::main] macro, but that doesn't actually have to parse the function body to do its work. syn-mid treats the body as an opaque blob, which should be a little faster. --- crates/neon-macros/Cargo.toml | 3 ++- crates/neon-macros/src/legacy.rs | 2 +- crates/neon-macros/src/napi.rs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/neon-macros/Cargo.toml b/crates/neon-macros/Cargo.toml index 4feca4985..2c52a583e 100644 --- a/crates/neon-macros/Cargo.toml +++ b/crates/neon-macros/Cargo.toml @@ -15,4 +15,5 @@ napi = [] [dependencies] quote = "1" -syn = { version = "1", features = ["full"] } +syn = "1" +syn-mid = "0.5" diff --git a/crates/neon-macros/src/legacy.rs b/crates/neon-macros/src/legacy.rs index f7a0a60e0..1abe7ccc9 100644 --- a/crates/neon-macros/src/legacy.rs +++ b/crates/neon-macros/src/legacy.rs @@ -2,7 +2,7 @@ pub(crate) fn main( _attr: proc_macro::TokenStream, item: proc_macro::TokenStream, ) -> proc_macro::TokenStream { - let input = syn::parse_macro_input!(item as syn::ItemFn); + let input = syn::parse_macro_input!(item as syn_mid::ItemFn); let attrs = &input.attrs; let vis = &input.vis; diff --git a/crates/neon-macros/src/napi.rs b/crates/neon-macros/src/napi.rs index ad2d4b868..530944eea 100644 --- a/crates/neon-macros/src/napi.rs +++ b/crates/neon-macros/src/napi.rs @@ -2,7 +2,7 @@ pub(crate) fn main( _attr: proc_macro::TokenStream, item: proc_macro::TokenStream, ) -> proc_macro::TokenStream { - let input = syn::parse_macro_input!(item as syn::ItemFn); + let input = syn::parse_macro_input!(item as syn_mid::ItemFn); let attrs = &input.attrs; let vis = &input.vis; From f23a3a0b965189e913db2c0ad497ed3510210ce1 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Thu, 28 Oct 2021 10:22:56 -0400 Subject: [PATCH 4/5] 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 5/5] 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))) } }