From 0d0affd5e9f01cccbb35f9d32a35d384c94126e6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Oct 2020 10:27:31 +0800 Subject: [PATCH] Eagerly evaluate built-in operators for OptimizationLevel::Simple. --- RELEASES.md | 4 +- doc/src/engine/optimize/eager.md | 18 +--- doc/src/engine/optimize/index.md | 82 +++++++++++------ doc/src/engine/optimize/optimize-levels.md | 5 +- doc/src/engine/optimize/reoptimize.md | 10 +- doc/src/engine/optimize/side-effects.md | 17 ++-- doc/src/links.md | 1 + examples/repl.rs | 2 +- src/fn_call.rs | 28 +++++- src/module/mod.rs | 17 ++++ src/optimize.rs | 102 ++++++++++++--------- 11 files changed, 182 insertions(+), 104 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 4aea4692..394e5e80 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -16,7 +16,9 @@ Breaking changes New features ------------ -* `is_def_var()` to detect if variable is defined and `is_def_fn()` to detect if script function is defined. +* `OptimizationLevel::Simple` now eagerly evaluates built-in binary operators of primary types (if not overloaded). +* Added `is_def_var()` to detect if variable is defined and `is_def_fn()` to detect if script function is defined. +* Added `Module::get_script_fn` to get a scripted function in a module, if any, based on name and number of parameters. Version 0.19.0 diff --git a/doc/src/engine/optimize/eager.md b/doc/src/engine/optimize/eager.md index b6bb26c8..2071154b 100644 --- a/doc/src/engine/optimize/eager.md +++ b/doc/src/engine/optimize/eager.md @@ -3,8 +3,8 @@ Eager Function Evaluation When Using Full Optimization Level {{#include ../../links.md}} -When the optimization level is [`OptimizationLevel::Full`], the [`Engine`] assumes all functions to be _pure_ and will _eagerly_ -evaluated all function calls with constant arguments, using the result to replace the call. +When the optimization level is [`OptimizationLevel::Full`], the [`Engine`] assumes all functions to be _pure_ +and will _eagerly_ evaluated all function calls with constant arguments, using the result to replace the call. This also applies to all operators (which are implemented as functions). @@ -14,8 +14,8 @@ For instance, the same example above: // When compiling the following with OptimizationLevel::Full... const DECISION = 1; - // this condition is now eliminated because 'DECISION == 1' -if DECISION == 1 { // is a function call to the '==' function, and it returns 'true' + // this condition is now eliminated because 'sign(DECISION) > 0' +if DECISION.sign() > 0 { // is a call to the 'sign' and '>' functions, and they return 'true' print("hello!"); // this block is promoted to the parent level } else { print("boo!"); // this block is eliminated because it is never reached @@ -24,13 +24,3 @@ if DECISION == 1 { // is a function call to the '==' function, and it r print("hello!"); // <- the above is equivalent to this // ('print' and 'debug' are handled specially) ``` - -Because of the eager evaluation of functions, many constant expressions will be evaluated and replaced by the result. -This does not happen with [`OptimizationLevel::Simple`] which doesn't assume all functions to be _pure_. - -```rust -// When compiling the following with OptimizationLevel::Full... - -let x = (1+2)*3-4/5%6; // <- will be replaced by 'let x = 9' -let y = (1>2) || (3<=4); // <- will be replaced by 'let y = true' -``` diff --git a/doc/src/engine/optimize/index.md b/doc/src/engine/optimize/index.md index e28aa748..6f293181 100644 --- a/doc/src/engine/optimize/index.md +++ b/doc/src/engine/optimize/index.md @@ -61,17 +61,63 @@ For fixed script texts, the constant values can be provided in a user-defined [` to the [`Engine`] for use in compilation and evaluation. -Watch Out for Function Calls ---------------------------- +Eager Operator Evaluations +------------------------- Beware, however, that most operators are actually function calls, and those functions can be overridden, -so they are not optimized away: +so whether they are optimized away depends on the situation: + +* If the operands are not _constant_ values, it is not optimized. + +* If the operator is [overloaded][operator overloading], it is not optimized because the overloading function may not be _pure_ + (i.e. may cause side-effects when called). + +* If the operator is not _binary_, it is not optimized. Only binary operators are built-in to Rhai. + +* If the operands are not of the same type, it is not optimized. + +* If the operator is not _built-in_ (see list of [built-in operators]), it is not optimized. + +* If the operator is a binary built-in operator for a [standard type][standard types], it is called and replaced by a constant result. + +Rhai guarantees that no external function will be run (in order not to trigger side-effects) during the +optimization process (unless the optimization level is set to [`OptimizationLevel::Full`]). ```rust -const DECISION = 1; +const DECISION = 1; // this is an integer, one of the standard types -if DECISION == 1 { // NOT optimized away because you can define - : // your own '==' function to override the built-in default! +if DECISION == 1 { // this is optimized into 'true' + : +} else if DECISION == 2 { // this is optimized into 'false' + : +} else if DECISION == 3 { // this is optimized into 'false' + : +} else { + : +} +``` + +Because of the eager evaluation of operators for [standard types], many constant expressions will be evaluated +and replaced by the result. + +```rust +let x = (1+2)*3-4/5%6; // will be replaced by 'let x = 9' + +let y = (1>2) || (3<=4); // will be replaced by 'let y = true' +``` + +For operators that are not optimized away due to one of the above reasons, the function calls +are simply left behind: + +```rust +// Assume 'new_state' returns some custom type that is NOT one of the standard types. +// Also assume that the '==; operator is defined for that custom type. +const DECISION_1 = new_state(1); +const DECISION_2 = new_state(2); +const DECISION_3 = new_state(3); + +if DECISION == 1 { // NOT optimized away because the operator is not built-in + : // and may cause side-effects if called! : } else if DECISION == 2 { // same here, NOT optimized away : @@ -82,28 +128,4 @@ if DECISION == 1 { // NOT optimized away because you can define } ``` -because no operator functions will be run (in order not to trigger side-effects) during the optimization process -(unless the optimization level is set to [`OptimizationLevel::Full`]). - -So, instead, do this: - -```rust -const DECISION_1 = true; -const DECISION_2 = false; -const DECISION_3 = false; - -if DECISION_1 { - : // this branch is kept and promoted to the parent level -} else if DECISION_2 { - : // this branch is eliminated -} else if DECISION_3 { - : // this branch is eliminated -} else { - : // this branch is eliminated -} -``` - -In general, boolean constants are most effective for the optimizer to automatically prune -large `if`-`else` branches because they do not depend on operators. - Alternatively, turn the optimizer to [`OptimizationLevel::Full`]. diff --git a/doc/src/engine/optimize/optimize-levels.md b/doc/src/engine/optimize/optimize-levels.md index 97dc27ea..0f43938b 100644 --- a/doc/src/engine/optimize/optimize-levels.md +++ b/doc/src/engine/optimize/optimize-levels.md @@ -8,9 +8,10 @@ There are three levels of optimization: `None`, `Simple` and `Full`. * `None` is obvious - no optimization on the AST is performed. * `Simple` (default) performs only relatively _safe_ optimizations without causing side-effects - (i.e. it only relies on static analysis and will not actually perform any function calls). + (i.e. it only relies on static analysis and [built-in operators] for constant [standard types], + and will not perform any external function calls). -* `Full` is _much_ more aggressive, _including_ running functions on constant arguments to determine their result. +* `Full` is _much_ more aggressive, _including_ calling external functions on constant arguments to determine their result. One benefit to this is that many more optimization opportunities arise, especially with regards to comparison operators. diff --git a/doc/src/engine/optimize/reoptimize.md b/doc/src/engine/optimize/reoptimize.md index e8292e4c..38726649 100644 --- a/doc/src/engine/optimize/reoptimize.md +++ b/doc/src/engine/optimize/reoptimize.md @@ -16,11 +16,11 @@ The final, optimized [`AST`] is then used for evaluations. // Compile master script to AST let master_ast = engine.compile( r" - if SCENARIO_1 { + if SCENARIO == 1 { do_work(); - } else if SCENARIO_2 { + } else if SCENARIO == 2 { do_something(); - } else if SCENARIO_3 { + } else if SCENARIO == 3 { do_something_else(); } else { do_nothing(); @@ -29,9 +29,7 @@ r" // Create a new 'Scope' - put constants in it to aid optimization let mut scope = Scope::new(); -scope.push_constant("SCENARIO_1", true); -scope.push_constant("SCENARIO_2", false); -scope.push_constant("SCENARIO_3", false); +scope.push_constant("SCENARIO", 1_i64); // Re-optimize the AST let new_ast = engine.optimize_ast(&scope, master_ast.clone(), OptimizationLevel::Simple); diff --git a/doc/src/engine/optimize/side-effects.md b/doc/src/engine/optimize/side-effects.md index 76009c55..fbacea0b 100644 --- a/doc/src/engine/optimize/side-effects.md +++ b/doc/src/engine/optimize/side-effects.md @@ -3,17 +3,20 @@ Side-Effect Considerations for Full Optimization Level {{#include ../../links.md}} -All of Rhai's built-in functions (and operators which are implemented as functions) are _pure_ (i.e. they do not mutate state -nor cause any side-effects, with the exception of `print` and `debug` which are handled specially) so using -[`OptimizationLevel::Full`] is usually quite safe _unless_ custom types and functions are registered. +All of Rhai's built-in functions (and operators which are implemented as functions) are _pure_ +(i.e. they do not mutate state nor cause any side-effects, with the exception of `print` and `debug` +which are handled specially) so using [`OptimizationLevel::Full`] is usually quite safe _unless_ +custom types and functions are registered. -If custom functions are registered, they _may_ be called (or maybe not, if the calls happen to lie within a pruned code block). +If custom functions are registered, they _may_ be called (or maybe not, if the calls happen to lie +within a pruned code block). -If custom functions are registered to overload built-in operators, they will also be called when the operators are used -(in an `if` statement, for example) causing side-effects. +If custom functions are registered to overload built-in operators, they will also be called when +the operators are used (in an `if` statement, for example) causing side-effects. Therefore, the rule-of-thumb is: * _Always_ register custom types and functions _after_ compiling scripts if [`OptimizationLevel::Full`] is used. -* _DO NOT_ depend on knowledge that the functions have no side-effects, because those functions can change later on and, when that happens, existing scripts may break in subtle ways. +* _DO NOT_ depend on knowledge that the functions have no side-effects, because those functions can change later on and, + when that happens, existing scripts may break in subtle ways. diff --git a/doc/src/links.md b/doc/src/links.md index 2fd9d152..ac7f1f4a 100644 --- a/doc/src/links.md +++ b/doc/src/links.md @@ -100,6 +100,7 @@ [function namespaces]: {{rootUrl}}/language/fn-namespaces.md [anonymous function]: {{rootUrl}}/language/fn-anon.md [anonymous functions]: {{rootUrl}}/language/fn-anon.md +[operator overloading]: {{rootUrl}}/rust/operators.md [`Module`]: {{rootUrl}}/language/modules/index.md [module]: {{rootUrl}}/language/modules/index.md diff --git a/examples/repl.rs b/examples/repl.rs index ee73c193..5a76b93c 100644 --- a/examples/repl.rs +++ b/examples/repl.rs @@ -146,7 +146,7 @@ fn main() { #[cfg(not(feature = "no_optimize"))] { - ast = engine.optimize_ast(&scope, r, OptimizationLevel::Full); + ast = engine.optimize_ast(&scope, r, OptimizationLevel::Simple); } #[cfg(feature = "no_optimize")] diff --git a/src/fn_call.rs b/src/fn_call.rs index b78e222f..473a948d 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -439,7 +439,33 @@ impl Engine { } // Has a system function an override? - fn has_override(&self, lib: &Module, hash_fn: u64, hash_script: u64, pub_only: bool) -> bool { + pub(crate) fn has_override_by_name_and_arguments( + &self, + lib: &Module, + name: &str, + arg_types: &[TypeId], + pub_only: bool, + ) -> bool { + let arg_len = if arg_types.is_empty() { + usize::MAX + } else { + arg_types.len() + }; + + let hash_fn = calc_fn_hash(empty(), name, arg_len, arg_types.iter().cloned()); + let hash_script = calc_fn_hash(empty(), name, arg_types.len(), empty()); + + self.has_override(lib, hash_fn, hash_script, pub_only) + } + + // Has a system function an override? + pub(crate) fn has_override( + &self, + lib: &Module, + hash_fn: u64, + hash_script: u64, + pub_only: bool, + ) -> bool { // NOTE: We skip script functions for global_module and packages, and native functions for lib // First check script-defined functions diff --git a/src/module/mod.rs b/src/module/mod.rs index e62ef520..10680b33 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -294,6 +294,23 @@ impl Module { hash_script } + /// Get a script-defined function in the module based on name and number of parameters. + pub fn get_script_fn( + &self, + name: &str, + num_params: usize, + public_only: bool, + ) -> Option<&Shared> { + self.functions + .values() + .find(|(fn_name, access, num, _, _)| { + (!public_only || *access == FnAccess::Public) + && *num == num_params + && fn_name == name + }) + .map(|(_, _, _, _, f)| f.get_shared_fn_def()) + } + /// Does a sub-module exist in the module? /// /// # Examples diff --git a/src/optimize.rs b/src/optimize.rs index 0e916878..c21423ab 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -5,6 +5,7 @@ use crate::calc_fn_hash; use crate::engine::{ Engine, KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF, }; +use crate::fn_call::run_builtin_binary_op; use crate::fn_native::FnPtr; use crate::module::Module; use crate::parser::{map_dynamic_to_expr, Expr, ScriptFnDef, Stmt, AST}; @@ -568,58 +569,75 @@ fn optimize_expr(expr: Expr, state: &mut State) -> Expr { } } + // Call built-in functions + Expr::FnCall(mut x) + if x.1.is_none() // Non-qualified + && state.optimization_level == OptimizationLevel::Simple // simple optimizations + && x.3.len() == 2 // binary call + && x.3.iter().all(Expr::is_constant) // all arguments are constants + => { + let ((name, _, _, pos), _, _, args, _) = x.as_mut(); + + let arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); + 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(state.lib, name, arg_types.as_ref(), false) { + if let Some(expr) = run_builtin_binary_op(name, &arg_values[0], &arg_values[1]) + .ok().flatten() + .and_then(|result| map_dynamic_to_expr(result, *pos)) + { + state.set_dirty(); + return expr; + } + } + + x.3 = x.3.into_iter().map(|a| optimize_expr(a, state)).collect(); + Expr::FnCall(x) + } + // Eagerly call functions Expr::FnCall(mut x) if x.1.is_none() // Non-qualified && state.optimization_level == OptimizationLevel::Full // full optimizations - && x.3.iter().all(|expr| expr.is_constant()) // all arguments are constants + && x.3.iter().all(Expr::is_constant) // all arguments are constants => { let ((name, _, _, pos), _, _, args, def_value) = x.as_mut(); - // First search in functions lib (can override built-in) - // Cater for both normal function call style and method call style (one additional arguments) - let has_script_fn = cfg!(not(feature = "no_function")) && state.lib.iter_fn().find(|(_, _, _, _,f)| { - if !f.is_script() { return false; } - let fn_def = f.get_fn_def(); - fn_def.name == name && (args.len()..=args.len() + 1).contains(&fn_def.params.len()) - }).is_some(); + // First search for script-defined functions (can override built-in) + let has_script_fn = cfg!(not(feature = "no_function")) + && state.lib.get_script_fn(name, args.len(), false).is_some(); - if has_script_fn { - // A script-defined function overrides the built-in function - do not make the call - x.3 = x.3.into_iter().map(|a| optimize_expr(a, state)).collect(); - return Expr::FnCall(x); + if !has_script_fn { + let mut arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); + + // Save the typename of the first argument if it is `type_of()` + // This is to avoid `call_args` being passed into the closure + let arg_for_type_of = if name == KEYWORD_TYPE_OF && arg_values.len() == 1 { + state.engine.map_type_name(arg_values[0].type_name()) + } else { + "" + }; + + if let Some(expr) = call_fn_with_constant_arguments(&state, name, arg_values.as_mut()) + .or_else(|| { + if !arg_for_type_of.is_empty() { + // Handle `type_of()` + Some(arg_for_type_of.to_string().into()) + } else { + // Otherwise use the default value, if any + def_value.map(|v| v.into()) + } + }) + .and_then(|result| map_dynamic_to_expr(result, *pos)) + { + state.set_dirty(); + return expr; + } } - let mut arg_values: StaticVec<_> = args.iter().map(Expr::get_constant_value).collect(); - - // Save the typename of the first argument if it is `type_of()` - // This is to avoid `call_args` being passed into the closure - let arg_for_type_of = if name == KEYWORD_TYPE_OF && arg_values.len() == 1 { - state.engine.map_type_name(arg_values[0].type_name()) - } else { - "" - }; - - call_fn_with_constant_arguments(&state, name, arg_values.as_mut()) - .or_else(|| { - if !arg_for_type_of.is_empty() { - // Handle `type_of()` - Some(arg_for_type_of.to_string().into()) - } else { - // Otherwise use the default value, if any - def_value.map(|v| v.into()) - } - }) - .and_then(|result| map_dynamic_to_expr(result, *pos)) - .map(|expr| { - state.set_dirty(); - expr - }) - .unwrap_or_else(|| { - // Optimize function call arguments - x.3 = x.3.into_iter().map(|a| optimize_expr(a, state)).collect(); - Expr::FnCall(x) - }) + x.3 = x.3.into_iter().map(|a| optimize_expr(a, state)).collect(); + Expr::FnCall(x) } // id(args ..) -> optimize function call arguments