Skip to content

Commit

Permalink
Merge pull request #534 from neon-bindings/kv/n-api-arg-cache
Browse files Browse the repository at this point in the history
Fix #530: Cache function arguments in N-API
  • Loading branch information
dherman authored Jul 12, 2020
2 parents 35c8b60 + 861b1ed commit d8c6775
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 19 deletions.
14 changes: 5 additions & 9 deletions crates/neon-runtime/src/napi/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,11 @@ pub unsafe extern "C" fn len(env: Env, info: FunctionCallbackInfo) -> i32 {
argc as i32
}

/// Mutates the `out` argument provided to refer to the `napi_value` of the `i`th argument
/// passed to the function.
pub unsafe extern "C" fn get(env: Env, info: FunctionCallbackInfo, i: i32, out: &mut Local) {
// TODO make this not allocate: https://github.com/neon-bindings/neon/issues/530
// Instead, we can probably get all the arguments at once in `neon` itself?
let mut args = vec![null_mut(); (i + 1) as usize];
/// Returns the function arguments as a `Vec<Local>`
pub unsafe extern "C" fn argv(env: Env, info: FunctionCallbackInfo) -> Vec<Local> {
let len = len(env, info);
let mut args = vec![null_mut(); len as usize];
let mut num_args = args.len();

let status = napi::napi_get_cb_info(
env,
info,
Expand All @@ -91,6 +88,5 @@ pub unsafe extern "C" fn get(env: Env, info: FunctionCallbackInfo, i: i32, out:
null_mut(),
);
assert_eq!(status, napi::napi_status::napi_ok);
assert!(num_args > i as usize);
*out = args[i as usize];
args
}
39 changes: 29 additions & 10 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ impl CallbackInfo<'_> {
}
}

#[cfg(feature = "legacy-runtime")]
pub fn get<'b, C: Context<'b>>(&self, cx: &mut C, i: i32) -> Option<Handle<'b, JsValue>> {
if i < 0 || i >= self.len(cx) {
return None;
Expand All @@ -72,14 +73,10 @@ impl CallbackInfo<'_> {
}
}

pub fn require<'b, C: Context<'b>>(&self, cx: &mut C, i: i32) -> JsResult<'b, JsValue> {
if i < 0 || i >= self.len(cx) {
return cx.throw_type_error("not enough arguments");
}
#[cfg(feature = "napi-runtime")]
pub fn argv<'b, C: Context<'b>>(&self, cx: &mut C) -> Vec<raw::Local> {
unsafe {
let mut local: raw::Local = std::mem::zeroed();
neon_runtime::call::get(cx.env().to_raw(), self.info, i, &mut local);
Ok(Handle::new_internal(JsValue::from_raw(local)))
neon_runtime::call::argv(cx.env().to_raw(), self.info)
}
}

Expand Down Expand Up @@ -450,6 +447,8 @@ impl<'a, 'b> Context<'a> for ComputeContext<'a, 'b> { }
pub struct CallContext<'a, T: This> {
scope: Scope<'a, raw::HandleScope>,
info: &'a CallbackInfo<'a>,
#[cfg(feature = "napi-runtime")]
arguments: Option<Vec<raw::Local>>,
phantom_type: PhantomData<T>
}

Expand All @@ -464,6 +463,8 @@ impl<'a, T: This> CallContext<'a, T> {
f(CallContext {
scope,
info,
#[cfg(feature = "napi-runtime")]
arguments: None,
phantom_type: PhantomData
})
})
Expand All @@ -474,13 +475,31 @@ impl<'a, T: This> CallContext<'a, T> {

/// Produces the `i`th argument, or `None` if `i` is greater than or equal to `self.len()`.
pub fn argument_opt(&mut self, i: i32) -> Option<Handle<'a, JsValue>> {
self.info.get(self, i)
#[cfg(feature = "legacy-runtime")]
{ self.info.get(self, i) }

#[cfg(feature = "napi-runtime")]
{
let local = if let Some(arguments) = &self.arguments {
arguments.get(i as usize).cloned()
} else {
let arguments = self.info.argv(self);
let local = arguments.get(i as usize).cloned();

self.arguments = Some(arguments);
local
};

local.map(|local| Handle::new_internal(JsValue::from_raw(local)))
}
}

/// Produces the `i`th argument and casts it to the type `V`, or throws an exception if `i` is greater than or equal to `self.len()` or cannot be cast to `V`.
pub fn argument<V: Value>(&mut self, i: i32) -> JsResult<'a, V> {
let a = self.info.require(self, i)?;
a.downcast_or_throw(self)
match self.argument_opt(i) {
Some(v) => v.downcast_or_throw(self),
None => self.throw_type_error("not enough arguments")
}
}

/// Produces a handle to the `this`-binding.
Expand Down

0 comments on commit d8c6775

Please sign in to comment.