From c5e2620d0f54a48b3aae5f637237345fcb23e778 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 3 Feb 2021 19:14:26 +0800 Subject: [PATCH 1/8] Minor code refactors. --- src/ast.rs | 2 +- src/fn_call.rs | 110 ++++++++++++++++++++++++------------------------- src/parser.rs | 74 ++++++++++++++------------------- 3 files changed, 88 insertions(+), 98 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 910352fe..78d179de 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1305,7 +1305,7 @@ impl Expr { _ => return None, }) } - /// Is the expression a simple variable access? + /// Return the variable name if the expression a simple variable access. #[inline(always)] pub(crate) fn get_variable_access(&self, non_qualified: bool) -> Option<&str> { match self { diff --git a/src/fn_call.rs b/src/fn_call.rs index 7e277d77..03d9e33a 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -914,59 +914,6 @@ impl Engine { ) -> Result> { let args_expr = args_expr.as_ref(); - // Handle Fn() - if fn_name == KEYWORD_FN_PTR && args_expr.len() == 1 { - let hash_fn = - calc_native_fn_hash(empty(), fn_name, once(TypeId::of::())); - - if !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) { - // Fn - only in function call style - return self - .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)? - .take_immutable_string() - .map_err(|typ| { - self.make_type_mismatch_err::(typ, args_expr[0].position()) - }) - .and_then(|s| FnPtr::try_from(s)) - .map(Into::::into) - .map_err(|err| err.fill_position(args_expr[0].position())); - } - } - - // Handle curry() - if fn_name == KEYWORD_FN_PTR_CURRY && args_expr.len() > 1 { - let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; - - if !fn_ptr.is::() { - return Err(self.make_type_mismatch_err::( - self.map_type_name(fn_ptr.type_name()), - args_expr[0].position(), - )); - } - - let (fn_name, mut fn_curry) = fn_ptr.cast::().take_data(); - - // Append the new curried arguments to the existing list. - - args_expr - .iter() - .skip(1) - .try_for_each(|expr| -> Result<(), Box> { - fn_curry.push(self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?); - Ok(()) - })?; - - return Ok(FnPtr::new_unchecked(fn_name, fn_curry).into()); - } - - // Handle is_shared() - #[cfg(not(feature = "no_closure"))] - if fn_name == crate::engine::KEYWORD_IS_SHARED && args_expr.len() == 1 { - let value = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; - - return Ok(value.is_shared().into()); - } - // Handle call() - Redirect function call let redirected; let mut args_expr = args_expr.as_ref(); @@ -1001,6 +948,58 @@ impl Engine { hash_script = calc_script_fn_hash(empty(), name, args_len); } + // Handle Fn() + if name == KEYWORD_FN_PTR && args_expr.len() == 1 { + let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::())); + + if !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) { + // Fn - only in function call style + return self + .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)? + .take_immutable_string() + .map_err(|typ| { + self.make_type_mismatch_err::(typ, args_expr[0].position()) + }) + .and_then(|s| FnPtr::try_from(s)) + .map(Into::::into) + .map_err(|err| err.fill_position(args_expr[0].position())); + } + } + + // Handle curry() + if name == KEYWORD_FN_PTR_CURRY && args_expr.len() > 1 { + let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; + + if !fn_ptr.is::() { + return Err(self.make_type_mismatch_err::( + self.map_type_name(fn_ptr.type_name()), + args_expr[0].position(), + )); + } + + let (name, mut fn_curry) = fn_ptr.cast::().take_data(); + + // Append the new curried arguments to the existing list. + + args_expr + .iter() + .skip(1) + .try_for_each(|expr| -> Result<(), Box> { + fn_curry.push(self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?); + Ok(()) + })?; + + return Ok(FnPtr::new_unchecked(name, fn_curry).into()); + } + + // Handle is_shared() + #[cfg(not(feature = "no_closure"))] + if name == crate::engine::KEYWORD_IS_SHARED && args_expr.len() == 1 { + let value = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; + + return Ok(value.is_shared().into()); + } + // Handle is_def_var() if name == KEYWORD_IS_DEF_VAR && args_expr.len() == 1 { let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::())); @@ -1055,8 +1054,9 @@ impl Engine { // No arguments args = Default::default(); } else { - // If the first argument is a variable, and there is no curried arguments, convert to method-call style - // in order to leverage potential &mut first argument and avoid cloning the value + // If the first argument is a variable, and there is no curried arguments, + // convert to method-call style in order to leverage potential &mut first argument and + // avoid cloning the value if curry.is_empty() && args_expr[0].get_variable_access(false).is_some() { // func(x, ...) -> x.func(...) arg_values = args_expr diff --git a/src/parser.rs b/src/parser.rs index 3b05f0ad..41c72a88 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1036,9 +1036,9 @@ fn parse_primary( match input.peek().unwrap().0 { // Function call Token::LeftParen | Token::Bang => { - // Once the identifier consumed we must enable next variables capturing #[cfg(not(feature = "no_closure"))] { + // Once the identifier consumed we must enable next variables capturing state.allow_capture = true; } let var_name_def = Ident { @@ -1050,9 +1050,9 @@ fn parse_primary( // Namespace qualification #[cfg(not(feature = "no_module"))] Token::DoubleColon => { - // Once the identifier consumed we must enable next variables capturing #[cfg(not(feature = "no_closure"))] { + // Once the identifier consumed we must enable next variables capturing state.allow_capture = true; } let var_name_def = Ident { @@ -1082,36 +1082,30 @@ fn parse_primary( match input.peek().unwrap().0 { // Function call is allowed to have reserved keyword - Token::LeftParen | Token::Bang => { - if is_keyword_function(&s) { - let var_name_def = Ident { - name: state.get_interned_string(s), - pos: settings.pos, - }; - Expr::Variable(Box::new((None, None, var_name_def))) - } else { - return Err(PERR::Reserved(s).into_err(settings.pos)); - } + Token::LeftParen | Token::Bang if is_keyword_function(&s) => { + let var_name_def = Ident { + name: state.get_interned_string(s), + pos: settings.pos, + }; + Expr::Variable(Box::new((None, None, var_name_def))) } - // Access to `this` as a variable is OK + // Access to `this` as a variable is OK within a function scope + _ if s == KEYWORD_THIS && settings.is_function_scope => { + let var_name_def = Ident { + name: state.get_interned_string(s), + pos: settings.pos, + }; + Expr::Variable(Box::new((None, None, var_name_def))) + } + // Cannot access to `this` as a variable not in a function scope _ if s == KEYWORD_THIS => { - if !settings.is_function_scope { - let msg = format!("'{}' can only be used in functions", s); - return Err(LexError::ImproperSymbol(s, msg).into_err(settings.pos)); - } else { - let var_name_def = Ident { - name: state.get_interned_string(s), - pos: settings.pos, - }; - Expr::Variable(Box::new((None, None, var_name_def))) - } + let msg = format!("'{}' can only be used in functions", s); + return Err(LexError::ImproperSymbol(s, msg).into_err(settings.pos)); } _ if is_valid_identifier(s.chars()) => { - return Err(PERR::Reserved(s).into_err(settings.pos)); - } - _ => { - return Err(LexError::UnexpectedInput(s).into_err(settings.pos)); + return Err(PERR::Reserved(s).into_err(settings.pos)) } + _ => return Err(LexError::UnexpectedInput(s).into_err(settings.pos)), } } @@ -1125,9 +1119,7 @@ fn parse_primary( } _ => { - return Err( - LexError::UnexpectedInput(token.syntax().to_string()).into_err(settings.pos) - ); + return Err(LexError::UnexpectedInput(token.syntax().to_string()).into_err(settings.pos)) } }; @@ -2275,6 +2267,12 @@ fn parse_let( (_, pos) => return Err(PERR::VariableExpected.into_err(pos)), }; + let name = state.get_interned_string(name); + let var_def = Ident { + name: name.clone(), + pos, + }; + // let name = ... let expr = if match_token(input, Token::Equals).0 { // let name = expr @@ -2283,21 +2281,13 @@ fn parse_let( None }; + state.stack.push((name, var_type)); + match var_type { // let name = expr - AccessMode::ReadWrite => { - let name = state.get_interned_string(name); - state.stack.push((name.clone(), AccessMode::ReadWrite)); - let var_def = Ident { name, pos }; - Ok(Stmt::Let(Box::new(var_def), expr, export, settings.pos)) - } + AccessMode::ReadWrite => Ok(Stmt::Let(Box::new(var_def), expr, export, settings.pos)), // const name = { expr:constant } - AccessMode::ReadOnly => { - let name = state.get_interned_string(name); - state.stack.push((name.clone(), AccessMode::ReadOnly)); - let var_def = Ident { name, pos }; - Ok(Stmt::Const(Box::new(var_def), expr, export, settings.pos)) - } + AccessMode::ReadOnly => Ok(Stmt::Const(Box::new(var_def), expr, export, settings.pos)), } } From f1c25628730eb4df4ae3deb413f621e805e7620e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 3 Feb 2021 19:22:35 +0800 Subject: [PATCH 2/8] Fix keyword typo. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 1d68fb03..927b7ad2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ include = [ "**/*.md", "Cargo.toml" ] -keywords = [ "scripting", "scripting-engine", "scripting language", "embedded" ] +keywords = [ "scripting", "scripting-engine", "scripting-language", "embedded" ] categories = [ "no-std", "embedded", "wasm", "parser-implementations" ] [dependencies] From 24ed5ef99ae003a887702ce0e505fdb677814bb8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 3 Feb 2021 19:23:50 +0800 Subject: [PATCH 3/8] Bump version. --- Cargo.toml | 2 +- RELEASES.md | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 927b7ad2..fd71d97c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ members = [ [package] name = "rhai" -version = "0.19.11" +version = "0.19.12" edition = "2018" authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung", "jhwgh1968"] description = "Embedded scripting for Rust" diff --git a/RELEASES.md b/RELEASES.md index f8bd9ca4..ab0e7ceb 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,6 +1,9 @@ Rhai Release Notes ================== +Version 0.19.12 +=============== + Version 0.19.11 =============== From dff124b2428e136ca29026225aaacefc9e4448b2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 6 Feb 2021 22:16:05 +0800 Subject: [PATCH 4/8] Save functions resolution cache during script call. --- src/fn_call.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 03d9e33a..e309a8db 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -392,10 +392,10 @@ impl Engine { // Merge in encapsulated environment, if any let mut lib_merged: StaticVec<_>; + let mut old_cache = None; let unified_lib = if let Some(ref env_lib) = fn_def.lib { - // If the library is modified, clear the functions lookup cache - state.functions_cache.clear(); + old_cache = Some(mem::take(&mut state.functions_cache)); lib_merged = Default::default(); lib_merged.push(env_lib.as_ref()); @@ -467,6 +467,10 @@ impl Engine { mods.truncate(prev_mods_len); state.scope_level = orig_scope_level; + if let Some(cache) = old_cache { + state.functions_cache = cache; + } + result } From a54b88a8b0edc50dd7afddfcba66cbee6eee34a6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 6 Feb 2021 22:16:44 +0800 Subject: [PATCH 5/8] Dynamic::into_shared not available under no_closure. --- RELEASES.md | 6 ++++++ src/dynamic.rs | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/RELEASES.md b/RELEASES.md index ab0e7ceb..6d20a5ba 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -4,6 +4,12 @@ Rhai Release Notes Version 0.19.12 =============== +Breaking changes +---------------- + +* `Dynamic::into_shared` is no longer available under `no_closure`. It used to panic. + + Version 0.19.11 =============== diff --git a/src/dynamic.rs b/src/dynamic.rs index c89c1250..c5d9f960 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -761,6 +761,8 @@ impl Dynamic { /// [`Arc`][std::sync::Arc]`<`[`RwLock`][std::sync::RwLock]`<`[`Dynamic`]`>>` /// depending on the `sync` feature. /// + /// Not available under [`no_closure`]. + /// /// Shared [`Dynamic`] values are relatively cheap to clone as they simply increment the /// reference counts. /// @@ -772,11 +774,11 @@ impl Dynamic { /// # Panics /// /// Panics under the `no_closure` feature. + #[cfg(not(feature = "no_closure"))] #[inline(always)] pub fn into_shared(self) -> Self { let _access = self.access_mode(); - #[cfg(not(feature = "no_closure"))] return match self.0 { Union::Shared(_, _) => self, _ => Self(Union::Shared(crate::Locked::new(self).into(), _access)), From 7b87f8185076f61db22144d266e247d303d3d572 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 7 Feb 2021 15:09:27 +0800 Subject: [PATCH 6/8] Add has_override and script calls to function resolution cache. --- src/fn_call.rs | 122 ++++++++++++++++++++++++++++----------- src/module/mod.rs | 3 +- src/optimize.rs | 2 +- src/packages/fn_basic.rs | 3 +- src/parser.rs | 9 +-- src/syntax.rs | 3 +- 6 files changed, 96 insertions(+), 46 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index e309a8db..e038e845 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -479,6 +479,7 @@ impl Engine { pub(crate) fn has_override_by_name_and_arguments( &self, mods: Option<&Imports>, + state: Option<&mut State>, lib: &[&Module], fn_name: &str, arg_types: impl AsRef<[TypeId]>, @@ -488,7 +489,7 @@ impl Engine { let hash_fn = calc_native_fn_hash(empty(), fn_name, arg_types.iter().cloned()); let hash_script = calc_script_fn_hash(empty(), fn_name, arg_types.len()); - self.has_override(mods, lib, hash_fn, hash_script, pub_only) + self.has_override(mods, state, lib, hash_fn, hash_script, pub_only) } // Has a system function an override? @@ -496,23 +497,54 @@ impl Engine { pub(crate) fn has_override( &self, mods: Option<&Imports>, + mut state: Option<&mut State>, lib: &[&Module], hash_fn: Option, hash_script: Option, pub_only: bool, ) -> bool { + // Check if it is already in the cache + if let Some(state) = state.as_mut() { + if let Some(hash) = hash_script { + match state.functions_cache.get(&hash) { + Some(v) => return v.is_some(), + None => (), + } + } + if let Some(hash) = hash_fn { + match state.functions_cache.get(&hash) { + Some(v) => return v.is_some(), + None => (), + } + } + } + // First check script-defined functions - hash_script.map(|hash| lib.iter().any(|&m| m.contains_fn(hash, pub_only))).unwrap_or(false) - //|| hash_fn.map(|hash| lib.iter().any(|&m| m.contains_fn(hash, pub_only))).unwrap_or(false) + let r = hash_script.map_or(false, |hash| lib.iter().any(|&m| m.contains_fn(hash, pub_only))) + //|| hash_fn.map_or(false, |hash| lib.iter().any(|&m| m.contains_fn(hash, pub_only))) // Then check registered functions - //|| hash_script.map(|hash| self.global_namespace.contains_fn(hash, pub_only)).unwrap_or(false) - || hash_fn.map(|hash| self.global_namespace.contains_fn(hash, false)).unwrap_or(false) + //|| hash_script.map_or(false, |hash| self.global_namespace.contains_fn(hash, pub_only)) + || hash_fn.map_or(false, |hash| self.global_namespace.contains_fn(hash, false)) // Then check packages - || hash_script.map(|hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false))).unwrap_or(false) - || hash_fn.map(|hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false))).unwrap_or(false) + || hash_script.map_or(false, |hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false))) + || hash_fn.map_or(false, |hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false))) // Then check imported modules - || hash_script.map(|hash| mods.map(|m| m.contains_fn(hash)).unwrap_or(false)).unwrap_or(false) - || hash_fn.map(|hash| mods.map(|m| m.contains_fn(hash)).unwrap_or(false)).unwrap_or(false) + || hash_script.map_or(false, |hash| mods.map_or(false, |m| m.contains_fn(hash))) + || hash_fn.map_or(false, |hash| mods.map_or(false, |m| m.contains_fn(hash))); + + // If there is no override, put that information into the cache + if !r { + if let Some(state) = state.as_mut() { + if let Some(hash) = hash_script { + state.functions_cache.insert(hash, None); + } + if let Some(hash) = hash_fn { + state.functions_cache.insert(hash, None); + } + } + } + + r } /// Perform an actual function call, native Rust or scripted, taking care of special functions. @@ -551,7 +583,14 @@ impl Engine { // type_of KEYWORD_TYPE_OF if args.len() == 1 - && !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) => + && !self.has_override( + Some(mods), + Some(state), + lib, + hash_fn, + hash_script, + pub_only, + ) => { Ok(( self.map_type_name(args[0].type_name()).to_string().into(), @@ -563,7 +602,14 @@ impl Engine { // by a function pointer so it isn't caught at parse time. KEYWORD_FN_PTR | KEYWORD_EVAL if args.len() == 1 - && !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) => + && !self.has_override( + Some(mods), + Some(state), + lib, + hash_fn, + hash_script, + pub_only, + ) => { EvalAltResult::ErrorRuntime( format!( @@ -579,25 +625,31 @@ impl Engine { // Script-like function found #[cfg(not(feature = "no_function"))] _ if hash_script.is_some() - && self.has_override(Some(mods), lib, None, hash_script, pub_only) => + && self.has_override(Some(mods), Some(state), lib, None, hash_script, pub_only) => { let hash_script = hash_script.unwrap(); - // Get function - let (func, mut source) = lib - .iter() - .find_map(|&m| { - m.get_fn(hash_script, pub_only) - .map(|f| (f, m.id_raw().cloned())) + // Check if function access already in the cache + let (func, source) = state + .functions_cache + .entry(hash_script) + .or_insert_with(|| { + lib.iter() + .find_map(|&m| { + m.get_fn(hash_script, pub_only) + .map(|f| (f.clone(), m.id_raw().cloned())) + }) + //.or_else(|| self.global_namespace.get_fn(hash_script, pub_only)) + .or_else(|| { + self.global_modules.iter().find_map(|m| { + m.get_fn(hash_script, false) + .map(|f| (f.clone(), m.id_raw().cloned())) + }) + }) + //.or_else(|| mods.iter().find_map(|(_, m)| m.get_qualified_fn(hash_script).map(|f| (f, m.id_raw().clone())))) }) - //.or_else(|| self.global_namespace.get_fn(hash_script, pub_only)) - .or_else(|| { - self.global_modules.iter().find_map(|m| { - m.get_fn(hash_script, false) - .map(|f| (f, m.id_raw().cloned())) - }) - }) - //.or_else(|| mods.iter().find_map(|(_, m)| m.get_qualified_fn(hash_script).map(|f| (f, m.id_raw().clone())))) + .as_ref() + .map(|(f, s)| (f.clone(), s.clone())) .unwrap(); assert!(func.is_script()); @@ -624,7 +676,8 @@ impl Engine { // Method call of script function - map first argument to `this` let (first, rest) = args.split_first_mut().unwrap(); - mem::swap(&mut state.source, &mut source); + let orig_source = mem::take(&mut state.source); + state.source = source; let level = _level + 1; @@ -641,7 +694,7 @@ impl Engine { ); // Restore the original source - state.source = source; + state.source = orig_source; result? } else { @@ -650,7 +703,8 @@ impl Engine { let mut backup: ArgBackup = Default::default(); backup.change_first_arg_to_copy(is_ref, args); - mem::swap(&mut state.source, &mut source); + let orig_source = mem::take(&mut state.source); + state.source = source; let level = _level + 1; @@ -658,7 +712,7 @@ impl Engine { .call_script_fn(scope, mods, state, lib, &mut None, func, args, pos, level); // Restore the original source - state.source = source; + state.source = orig_source; // Restore the original reference backup.restore_first_arg(args); @@ -926,7 +980,7 @@ impl Engine { if name == KEYWORD_FN_PTR_CALL && args_expr.len() >= 1 - && !self.has_override(Some(mods), lib, None, hash_script, pub_only) + && !self.has_override(Some(mods), Some(state), lib, None, hash_script, pub_only) { let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; @@ -956,7 +1010,7 @@ impl Engine { if name == KEYWORD_FN_PTR && args_expr.len() == 1 { let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::())); - if !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) { + if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script, pub_only) { // Fn - only in function call style return self .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)? @@ -1008,7 +1062,7 @@ impl Engine { if name == KEYWORD_IS_DEF_VAR && args_expr.len() == 1 { let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::())); - if !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) { + if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script, pub_only) { let var_name = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?; let var_name = var_name.as_str().map_err(|err| { @@ -1022,7 +1076,7 @@ impl Engine { if name == KEYWORD_EVAL && args_expr.len() == 1 { let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::())); - if !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) { + if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script, pub_only) { // eval - only in function call style let prev_len = scope.len(); let script = diff --git a/src/module/mod.rs b/src/module/mod.rs index 64788b74..35e12393 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -619,8 +619,7 @@ impl Module { if public_only { self.functions .get(&hash_fn) - .map(|FuncInfo { access, .. }| access.is_public()) - .unwrap_or(false) + .map_or(false, |FuncInfo { access, .. }| access.is_public()) } else { self.functions.contains_key(&hash_fn) } diff --git a/src/optimize.rs b/src/optimize.rs index 6a0864f0..7a7296b4 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -674,7 +674,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect(); // Search for overloaded operators (can override built-in). - if !state.engine.has_override_by_name_and_arguments(Some(&state.mods), state.lib, x.name.as_ref(), arg_types.as_ref(), false) { + if !state.engine.has_override_by_name_and_arguments(Some(&state.mods), None, state.lib, x.name.as_ref(), arg_types.as_ref(), false) { if let Some(result) = run_builtin_binary_op(x.name.as_ref(), &arg_values[0], &arg_values[1]) .ok().flatten() .and_then(|result| map_dynamic_to_expr(result, *pos)) diff --git a/src/packages/fn_basic.rs b/src/packages/fn_basic.rs index ad1586ab..c2943498 100644 --- a/src/packages/fn_basic.rs +++ b/src/packages/fn_basic.rs @@ -31,8 +31,9 @@ mod fn_ptr_functions { false } else { let hash_script = calc_script_fn_hash(empty(), fn_name, num_params as usize); + ctx.engine() - .has_override(ctx.mods, ctx.lib, None, hash_script, true) + .has_override(ctx.mods, None, ctx.lib, None, hash_script, true) } } } diff --git a/src/parser.rs b/src/parser.rs index 41c72a88..dd4b5812 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1731,8 +1731,7 @@ fn parse_binary_op( .engine .custom_keywords .get(c) - .map(Option::is_some) - .unwrap_or(false) + .map_or(false, Option::is_some) { state.engine.custom_keywords.get(c).unwrap().unwrap().get() } else { @@ -1763,8 +1762,7 @@ fn parse_binary_op( .engine .custom_keywords .get(c) - .map(Option::is_some) - .unwrap_or(false) + .map_or(false, Option::is_some) { state.engine.custom_keywords.get(c).unwrap().unwrap().get() } else { @@ -1875,8 +1873,7 @@ fn parse_binary_op( .engine .custom_keywords .get(&s) - .map(Option::is_some) - .unwrap_or(false) => + .map_or(false, Option::is_some) => { let hash_script = if is_valid_identifier(s.chars()) { // Accept non-native functions for custom operators diff --git a/src/syntax.rs b/src/syntax.rs index 78ce64f7..1ec40876 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -149,8 +149,7 @@ impl Engine { s if segments.is_empty() && token .as_ref() - .map(|v| v.is_keyword() || v.is_reserved()) - .unwrap_or(false) => + .map_or(false, |v| v.is_keyword() || v.is_reserved()) => { return Err(LexError::ImproperSymbol( s.to_string(), From aafff4fb9302b7abc3dbb8b78ff25a2c0919e25e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 7 Feb 2021 15:41:40 +0800 Subject: [PATCH 7/8] Use stacked functions resolution caches to further improve performance. --- RELEASES.md | 5 +++ src/engine.rs | 32 +++++++++++++----- src/fn_call.rs | 89 +++++++++++++++++++++++++++++++++----------------- 3 files changed, 87 insertions(+), 39 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 6d20a5ba..2d23c944 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -9,6 +9,11 @@ Breaking changes * `Dynamic::into_shared` is no longer available under `no_closure`. It used to panic. +Enhancements +------------ + +* Functions resolution cache is used in more cases, making repeated function calls faster. + Version 0.19.11 =============== diff --git a/src/engine.rs b/src/engine.rs index 21f65fa3..d7e6bfd5 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -520,10 +520,12 @@ pub struct State { #[cfg(not(feature = "no_module"))] pub resolver: Option>, /// Cached lookup values for function hashes. - pub functions_cache: HashMap< - NonZeroU64, - Option<(CallableFunction, Option)>, - StraightHasherBuilder, + pub functions_caches: StaticVec< + HashMap< + NonZeroU64, + Option<(CallableFunction, Option)>, + StraightHasherBuilder, + >, >, } @@ -1856,6 +1858,7 @@ impl Engine { statements: impl IntoIterator, level: usize, ) -> Result> { + let mut has_imports = false; let prev_always_search = state.always_search; let prev_scope_len = scope.len(); let prev_mods_len = mods.len(); @@ -1864,13 +1867,26 @@ impl Engine { let result = statements .into_iter() .try_fold(Default::default(), |_, stmt| { + match stmt { + Stmt::Import(_, _, _) => { + // When imports list is modified, clear the functions lookup cache + if has_imports { + state.functions_caches.last_mut().map(|c| c.clear()); + } else { + state.functions_caches.push(Default::default()); + } + has_imports = true; + } + _ => (), + } + self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level) }); scope.rewind(prev_scope_len); - if mods.len() != prev_mods_len { - // If imports list is modified, clear the functions lookup cache - state.functions_cache.clear(); + if has_imports { + // If imports list is modified, pop the functions lookup cache + state.functions_caches.pop(); } mods.truncate(prev_mods_len); state.scope_level -= 1; @@ -2365,8 +2381,6 @@ impl Engine { } else { mods.push(name_def.name.clone(), module); } - // When imports list is modified, clear the functions lookup cache - state.functions_cache.clear(); } state.modules += 1; diff --git a/src/fn_call.rs b/src/fn_call.rs index e038e845..15307458 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -176,28 +176,37 @@ impl Engine { self.inc_operations(state, pos)?; // Check if function access already in the cache - let func = &*state.functions_cache.entry(hash_fn).or_insert_with(|| { - // Search for the native function - // First search registered functions (can override packages) - // Then search packages - // Finally search modules + if state.functions_caches.is_empty() { + state.functions_caches.push(Default::default()); + } - //lib.get_fn(hash_fn, pub_only) - self.global_namespace - .get_fn(hash_fn, pub_only) - .cloned() - .map(|f| (f, None)) - .or_else(|| { - self.global_modules.iter().find_map(|m| { - m.get_fn(hash_fn, false) - .map(|f| (f.clone(), m.id_raw().cloned())) + let func = &*state + .functions_caches + .last_mut() + .unwrap() + .entry(hash_fn) + .or_insert_with(|| { + // Search for the native function + // First search registered functions (can override packages) + // Then search packages + // Finally search modules + + //lib.get_fn(hash_fn, pub_only) + self.global_namespace + .get_fn(hash_fn, pub_only) + .cloned() + .map(|f| (f, None)) + .or_else(|| { + self.global_modules.iter().find_map(|m| { + m.get_fn(hash_fn, false) + .map(|f| (f.clone(), m.id_raw().cloned())) + }) }) - }) - .or_else(|| { - mods.get_fn(hash_fn) - .map(|(f, source)| (f.clone(), source.cloned())) - }) - }); + .or_else(|| { + mods.get_fn(hash_fn) + .map(|(f, source)| (f.clone(), source.cloned())) + }) + }); if let Some((func, source)) = func { assert!(func.is_native()); @@ -392,11 +401,11 @@ impl Engine { // Merge in encapsulated environment, if any let mut lib_merged: StaticVec<_>; - let mut old_cache = None; + let mut unified = false; let unified_lib = if let Some(ref env_lib) = fn_def.lib { - old_cache = Some(mem::take(&mut state.functions_cache)); - + unified = true; + state.functions_caches.push(Default::default()); lib_merged = Default::default(); lib_merged.push(env_lib.as_ref()); lib_merged.extend(lib.iter().cloned()); @@ -467,8 +476,8 @@ impl Engine { mods.truncate(prev_mods_len); state.scope_level = orig_scope_level; - if let Some(cache) = old_cache { - state.functions_cache = cache; + if unified { + state.functions_caches.pop(); } result @@ -506,13 +515,13 @@ impl Engine { // Check if it is already in the cache if let Some(state) = state.as_mut() { if let Some(hash) = hash_script { - match state.functions_cache.get(&hash) { + match state.functions_caches.last().map_or(None, |c| c.get(&hash)) { Some(v) => return v.is_some(), None => (), } } if let Some(hash) = hash_fn { - match state.functions_cache.get(&hash) { + match state.functions_caches.last().map_or(None, |c| c.get(&hash)) { Some(v) => return v.is_some(), None => (), } @@ -536,10 +545,24 @@ impl Engine { if !r { if let Some(state) = state.as_mut() { if let Some(hash) = hash_script { - state.functions_cache.insert(hash, None); + if state.functions_caches.is_empty() { + state.functions_caches.push(Default::default()); + } + state + .functions_caches + .last_mut() + .unwrap() + .insert(hash, None); } if let Some(hash) = hash_fn { - state.functions_cache.insert(hash, None); + if state.functions_caches.is_empty() { + state.functions_caches.push(Default::default()); + } + state + .functions_caches + .last_mut() + .unwrap() + .insert(hash, None); } } } @@ -630,8 +653,14 @@ impl Engine { let hash_script = hash_script.unwrap(); // Check if function access already in the cache + if state.functions_caches.is_empty() { + state.functions_caches.push(Default::default()); + } + let (func, source) = state - .functions_cache + .functions_caches + .last_mut() + .unwrap() .entry(hash_script) .or_insert_with(|| { lib.iter() From f388d22c0fcfce63ad17278a5ec140af0ad11cc2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 7 Feb 2021 15:52:06 +0800 Subject: [PATCH 8/8] Fix no_module build., --- src/engine.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/engine.rs b/src/engine.rs index d7e6bfd5..2b0e6569 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1868,6 +1868,7 @@ impl Engine { .into_iter() .try_fold(Default::default(), |_, stmt| { match stmt { + #[cfg(not(feature = "no_module"))] Stmt::Import(_, _, _) => { // When imports list is modified, clear the functions lookup cache if has_imports {