From 1f815f995f66b30d9f3092a38df5d93190d3d42e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 11 Dec 2022 16:55:30 +0800 Subject: [PATCH] Speed up FnPtr::call when there is a linked scripted function. --- src/eval/expr.rs | 5 +- src/func/call.rs | 135 +++++++++---------- src/func/native.rs | 6 +- src/packages/array_basic.rs | 250 ++++++++++-------------------------- src/parser.rs | 3 +- src/types/fn_ptr.rs | 55 ++++++-- 6 files changed, 186 insertions(+), 268 deletions(-) diff --git a/src/eval/expr.rs b/src/eval/expr.rs index b0e6d518..d6981151 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -158,7 +158,10 @@ impl Engine { if let Some(fn_def) = global.lib.iter().flat_map(|m| m.iter_script_fn()).find_map( |(_, _, f, _, func)| if f == v.3.as_str() { Some(func) } else { None }, ) { - let val: Dynamic = crate::FnPtr::from(fn_def.clone()).into(); + let mut fn_ptr = + crate::FnPtr::new_unchecked(v.3.clone(), crate::StaticVec::new_const()); + fn_ptr.set_fn_def(Some(fn_def.clone())); + let val: Dynamic = fn_ptr.into(); return Ok(val.into()); } diff --git a/src/func/call.rs b/src/func/call.rs index 884a3d33..5b738743 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -761,20 +761,22 @@ impl Engine { // Linked to scripted function? #[cfg(not(feature = "no_function"))] if let Some(fn_def) = fn_ptr.fn_def() { - let mut this_ptr = Dynamic::NULL; + if fn_def.params.len() == args.len() { + let mut this_ptr = Dynamic::NULL; - return self - .call_script_fn( - global, - caches, - &mut Scope::new(), - &mut this_ptr, - fn_def, - args, - true, - fn_call_pos, - ) - .map(|v| (v, false)); + return self + .call_script_fn( + global, + caches, + &mut Scope::new(), + &mut this_ptr, + fn_def, + args, + true, + fn_call_pos, + ) + .map(|v| (v, false)); + } } #[cfg(not(feature = "no_function"))] @@ -827,7 +829,7 @@ impl Engine { }; #[cfg(feature = "no_function")] let (fn_name, is_anon, fn_curry) = { - let (fn_name, fn_curry, fn_def) = fn_ptr.take_data(); + let (fn_name, fn_curry) = fn_ptr.take_data(); (fn_name, false, fn_curry) }; @@ -843,22 +845,24 @@ impl Engine { // Linked to scripted function? #[cfg(not(feature = "no_function"))] if let Some(fn_def) = fn_def { - // Check for data race. - #[cfg(not(feature = "no_closure"))] - ensure_no_data_race(&fn_def.name, args, false)?; + if fn_def.params.len() == args.len() { + // Check for data race. + #[cfg(not(feature = "no_closure"))] + ensure_no_data_race(&fn_def.name, args, false)?; - return self - .call_script_fn( - global, - caches, - &mut Scope::new(), - target, - &fn_def, - args, - true, - fn_call_pos, - ) - .map(|v| (v, false)); + return self + .call_script_fn( + global, + caches, + &mut Scope::new(), + target, + &fn_def, + args, + true, + fn_call_pos, + ) + .map(|v| (v, false)); + } } // Add the first argument with the object pointer @@ -895,26 +899,14 @@ impl Engine { return Err(self.make_type_mismatch_err::(typ, fn_call_pos)); } - let fn_ptr = target.read_lock::().expect("`FnPtr`"); + let mut fn_ptr = target.read_lock::().expect("`FnPtr`").clone(); - // Curry call - Ok(( - if call_args.is_empty() { - fn_ptr.clone() - } else { - FnPtr::new_unchecked( - fn_ptr.fn_name_raw().clone(), - fn_ptr - .curry() - .iter() - .cloned() - .chain(call_args.iter_mut().map(mem::take)) - .collect(), - ) - } - .into(), - false, - )) + // Append the new curried arguments to the existing list. + call_args.iter_mut().map(mem::take).for_each(|value| { + fn_ptr.add_curry(value); + }); + + Ok((fn_ptr.into(), false)) } // Handle is_shared() @@ -1045,7 +1037,7 @@ impl Engine { }; #[cfg(feature = "no_function")] let (fn_name, is_anon, fn_curry) = { - let (fn_name, fn_curry, fn_def) = fn_ptr.take_data(); + let (fn_name, fn_curry) = fn_ptr.take_data(); (fn_name, false, fn_curry) }; @@ -1054,29 +1046,30 @@ impl Engine { // Linked to scripted function? #[cfg(not(feature = "no_function"))] if let Some(fn_def) = fn_def { - // Evaluate arguments - let mut arg_values = curry - .into_iter() - .map(Ok) - .chain(a_expr.iter().map(|expr| -> Result<_, RhaiError> { - self.get_arg_value(global, caches, scope, this_ptr, expr) - .map(|(v, ..)| v) - })) - .collect::>>()?; - let args = &mut arg_values.iter_mut().collect::>(); + if fn_def.params.len() == curry.len() + a_expr.len() { + // Evaluate arguments + let mut arg_values = curry + .into_iter() + .map(Ok) + .chain(a_expr.iter().map(|expr| -> Result<_, RhaiError> { + self.get_arg_value(global, caches, scope, this_ptr, expr) + .map(|(v, ..)| v) + })) + .collect::>>()?; + let args = &mut arg_values.iter_mut().collect::>(); + let mut this_ptr = Dynamic::NULL; - let mut this_ptr = Dynamic::NULL; - - return self.call_script_fn( - global, - caches, - &mut Scope::new(), - &mut this_ptr, - &fn_def, - args, - true, - pos, - ); + return self.call_script_fn( + global, + caches, + &mut Scope::new(), + &mut this_ptr, + &fn_def, + args, + true, + pos, + ); + } } // Redirect function name diff --git a/src/func/native.rs b/src/func/native.rs index 1e1270aa..8066a876 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -422,15 +422,15 @@ impl<'a> NativeCallContext<'a> { is_ref_mut: bool, is_method_call: bool, ) -> RhaiResult { - let mut global = &mut self.global.clone(); + let global = &mut self.global.clone(); + global.level += 1; + let caches = &mut Caches::new(); let fn_name = fn_name.as_ref(); let op_token = Token::lookup_symbol_from_syntax(fn_name).unwrap_or(NO_TOKEN); let args_len = args.len(); - global.level += 1; - if native_only { return self .engine() diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index e9cefb23..a490b8de 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -24,6 +24,49 @@ def_package! { } } +/// Make a call to a function pointer. +/// +/// If the function pointer is linked to a scripted function definition, use the appropriate number +/// of arguments to call it directly (one version attaches an extra numeric argument). +fn make_dual_arity_fn_ptr_call( + fn_name: &str, + fn_ptr: &FnPtr, + ctx: &NativeCallContext, + items: [Dynamic; N], + number: usize, +) -> RhaiResult { + let arity = fn_ptr.fn_def().map(|f| f.params.len()).unwrap_or(0); + + if arity == N { + fn_ptr.call_raw(&ctx, None, items) + } else if arity == N + 1 { + let mut items2 = crate::StaticVec::new_const(); + items2.extend(IntoIterator::into_iter(items)); + items2.push((number as INT).into()); + fn_ptr.call_raw(&ctx, None, items2) + } else { + fn_ptr + .call_raw(&ctx, None, items.clone()) + .or_else(|err| match *err { + ERR::ErrorFunctionNotFound(sig, ..) if sig.starts_with(fn_ptr.fn_name()) => { + let mut items2 = crate::StaticVec::new_const(); + items2.extend(IntoIterator::into_iter(items)); + items2.push((number as INT).into()); + fn_ptr.call_raw(&ctx, None, items2) + } + _ => Err(err), + }) + .map_err(|err| { + Box::new(ERR::ErrorInFunctionCall( + fn_name.to_string(), + ctx.source().unwrap_or("").to_string(), + err, + Position::NONE, + )) + }) + } +} + #[export_module] pub mod array_functions { /// Number of elements in the array. @@ -658,26 +701,13 @@ pub mod array_functions { let mut ar = Array::with_capacity(array.len()); for (i, item) in array.into_iter().enumerate() { - ar.push( - mapper - .call_raw(&ctx, None, [item.clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(mapper.fn_name()) => - { - mapper.call_raw(&ctx, None, [item, (i as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "map".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })?, - ); + ar.push(make_dual_arity_fn_ptr_call( + "map", + &mapper, + &ctx, + [item], + i, + )?); } Ok(ar) @@ -748,24 +778,7 @@ pub mod array_functions { let mut ar = Array::new(); for (i, item) in array.into_iter().enumerate() { - if filter - .call_raw(&ctx, None, [item.clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(filter.fn_name()) => - { - filter.call_raw(&ctx, None, [item.clone(), (i as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "filter".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })? + if make_dual_arity_fn_ptr_call("filter", &filter, &ctx, [item.clone()], i)? .as_bool() .unwrap_or(false) { @@ -1058,24 +1071,7 @@ pub mod array_functions { let (start, ..) = calc_offset_len(array.len(), start, 0); for (i, item) in array.iter().enumerate().skip(start) { - if filter - .call_raw(&ctx, None, [item.clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(filter.fn_name()) => - { - filter.call_raw(&ctx, None, [item.clone(), (i as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "index_of".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })? + if make_dual_arity_fn_ptr_call("index_of", &filter, &ctx, [item.clone()], i)? .as_bool() .unwrap_or(false) { @@ -1157,24 +1153,7 @@ pub mod array_functions { } for (i, item) in array.iter().enumerate() { - if filter - .call_raw(&ctx, None, [item.clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(filter.fn_name()) => - { - filter.call_raw(&ctx, None, [item.clone(), (i as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "some".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })? + if make_dual_arity_fn_ptr_call("some", &filter, &ctx, [item.clone()], i)? .as_bool() .unwrap_or(false) { @@ -1244,24 +1223,7 @@ pub mod array_functions { } for (i, item) in array.iter().enumerate() { - if !filter - .call_raw(&ctx, None, [item.clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(filter.fn_name()) => - { - filter.call_raw(&ctx, None, [item.clone(), (i as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "all".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })? + if !make_dual_arity_fn_ptr_call("all", &filter, &ctx, [item.clone()], i)? .as_bool() .unwrap_or(false) { @@ -1482,32 +1444,12 @@ pub mod array_functions { return Ok(initial); } - let mut result = initial; - - for (i, item) in array.iter().enumerate() { - let item = item.clone(); - - result = reducer - .call_raw(&ctx, None, [result.clone(), item.clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(reducer.fn_name()) => - { - reducer.call_raw(&ctx, None, [result, item, (i as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "reduce".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })?; - } - - Ok(result) + array + .iter() + .enumerate() + .try_fold(initial, |result, (i, item)| { + make_dual_arity_fn_ptr_call("reduce", &reducer, &ctx, [result, item.clone()], i) + }) } /// Reduce an array by iterating through all elements while applying a function named by `reducer`. /// @@ -1643,33 +1585,13 @@ pub mod array_functions { return Ok(initial); } - let mut result = initial; - let len = array.len(); - - for (i, item) in array.iter().rev().enumerate() { - let item = item.clone(); - - result = reducer - .call_raw(&ctx, None, [result.clone(), item.clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(reducer.fn_name()) => - { - reducer.call_raw(&ctx, None, [result, item, ((len - 1 - i) as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "reduce_rev".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })?; - } - - Ok(result) + array + .iter() + .rev() + .enumerate() + .try_fold(initial, |result, (i, item)| { + make_dual_arity_fn_ptr_call("reduce_rev", &reducer, &ctx, [result, item.clone()], array.len() - 1 - i) + }) } /// Reduce an array by iterating through all elements, in _reverse_ order, /// while applying a function named by `reducer`. @@ -1928,24 +1850,7 @@ pub mod array_functions { let mut x = 0; while x < array.len() { - if filter - .call_raw(&ctx, None, [array[x].clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(filter.fn_name()) => - { - filter.call_raw(&ctx, None, [array[x].clone(), (i as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "drain".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })? + if make_dual_arity_fn_ptr_call("drain", &filter, &ctx, [array[x].clone()], i)? .as_bool() .unwrap_or(false) { @@ -2124,24 +2029,7 @@ pub mod array_functions { let mut x = 0; while x < array.len() { - if filter - .call_raw(&ctx, None, [array[x].clone()]) - .or_else(|err| match *err { - ERR::ErrorFunctionNotFound(fn_sig, ..) - if fn_sig.starts_with(filter.fn_name()) => - { - filter.call_raw(&ctx, None, [array[x].clone(), (i as INT).into()]) - } - _ => Err(err), - }) - .map_err(|err| { - Box::new(ERR::ErrorInFunctionCall( - "retain".to_string(), - ctx.source().unwrap_or("").to_string(), - err, - Position::NONE, - )) - })? + if make_dual_arity_fn_ptr_call("retain", &filter, &ctx, [array[x].clone()], i)? .as_bool() .unwrap_or(false) { diff --git a/src/parser.rs b/src/parser.rs index a046283f..92da6876 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3798,7 +3798,8 @@ impl Engine { comments: Box::default(), }); - let fn_ptr = crate::FnPtr::from(script.clone()); + let mut fn_ptr = crate::FnPtr::new_unchecked(fn_name, StaticVec::new_const()); + fn_ptr.set_fn_def(Some(script.clone())); let expr = Expr::DynamicConstant(Box::new(fn_ptr.into()), settings.pos); #[cfg(not(feature = "no_closure"))] diff --git a/src/types/fn_ptr.rs b/src/types/fn_ptr.rs index 76b8f52c..870b914e 100644 --- a/src/types/fn_ptr.rs +++ b/src/types/fn_ptr.rs @@ -1,11 +1,11 @@ //! The `FnPtr` type. -use crate::eval::GlobalRuntimeState; +use crate::eval::{Caches, GlobalRuntimeState}; use crate::tokenizer::is_valid_function_name; use crate::types::dynamic::Variant; use crate::{ - Dynamic, Engine, FuncArgs, ImmutableString, NativeCallContext, Position, RhaiError, RhaiResult, - RhaiResultOf, StaticVec, AST, ERR, + Dynamic, Engine, FnArgsVec, FuncArgs, ImmutableString, NativeCallContext, Position, RhaiError, + RhaiResult, RhaiResultOf, StaticVec, AST, ERR, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -266,21 +266,45 @@ impl FnPtr { let mut args_data; if self.is_curried() { - args_data = StaticVec::with_capacity(self.curry().len() + arg_values.len()); + args_data = FnArgsVec::with_capacity(self.curry().len() + arg_values.len()); args_data.extend(self.curry().iter().cloned()); args_data.extend(arg_values.iter_mut().map(mem::take)); arg_values = &mut *args_data; }; - let is_method = this_ptr.is_some(); - - let mut args = StaticVec::with_capacity(arg_values.len() + 1); - if let Some(obj) = this_ptr { - args.push(obj); - } + let args = &mut StaticVec::with_capacity(arg_values.len() + 1); args.extend(arg_values.iter_mut()); - context.call_fn_raw(self.fn_name(), is_method, is_method, &mut args) + // Linked to scripted function? + #[cfg(not(feature = "no_function"))] + if let Some(fn_def) = self.fn_def() { + if fn_def.params.len() == args.len() { + let global = &mut context.global_runtime_state().clone(); + global.level += 1; + + let caches = &mut Caches::new(); + let mut null_ptr = Dynamic::NULL; + + return context.engine().call_script_fn( + global, + caches, + &mut crate::Scope::new(), + this_ptr.unwrap_or(&mut null_ptr), + &fn_def, + args, + true, + context.position(), + ); + } + } + + let is_method = this_ptr.is_some(); + + if let Some(obj) = this_ptr { + args.insert(0, obj); + } + + context.call_fn_raw(self.fn_name(), is_method, is_method, args) } /// Get a reference to the linked [`ScriptFnDef`][crate::ast::ScriptFnDef]. #[cfg(not(feature = "no_function"))] @@ -289,6 +313,15 @@ impl FnPtr { pub(crate) fn fn_def(&self) -> Option<&crate::Shared> { self.fn_def.as_ref() } + /// Set a reference to the linked [`ScriptFnDef`][crate::ast::ScriptFnDef]. + #[cfg(not(feature = "no_function"))] + #[inline(always)] + pub(crate) fn set_fn_def( + &mut self, + value: Option>>, + ) { + self.fn_def = value.map(Into::into); + } } impl fmt::Display for FnPtr {