Remove constants in function call expressions.

This commit is contained in:
Stephen Chung 2022-03-05 12:06:47 +08:00
parent 0335035b0f
commit e06c2b2abb
7 changed files with 77 additions and 124 deletions

View File

@ -28,6 +28,7 @@ Enhancements
* Variable definitions are optimized so that shadowed variables are reused as much as possible to reduce memory consumption.
* `FnAccess` and `FnNamespace` now implement `Ord` and `PartialOrd`.
* The `event_handler_map` example is enhanced to prevent shadowing of the state object map.
* Separation of constants in function calls is removed as its performance benefit is dubious.
Version 1.5.0

View File

@ -176,17 +176,6 @@ pub struct FnCallExpr {
pub hashes: FnCallHashes,
/// List of function call argument expressions.
pub args: StaticVec<Expr>,
/// List of function call arguments that are constants.
///
/// Any arguments in `args` that is [`Expr::Stack`] indexes into this
/// array to find the constant for use as its argument value.
///
/// # Notes
///
/// Constant arguments are very common in function calls, and keeping each constant in
/// an [`Expr::DynamicConstant`] involves an additional allocation. Keeping the constant
/// values in an inlined array avoids these extra allocations.
pub constants: StaticVec<Dynamic>,
/// Does this function call capture the parent scope?
pub capture_parent_scope: bool,
/// [Position] of the function name.
@ -206,9 +195,6 @@ impl fmt::Debug for FnCallExpr {
ff.field("hash", &self.hashes)
.field("name", &self.name)
.field("args", &self.args);
if !self.constants.is_empty() {
ff.field("constants", &self.constants);
}
ff.field("pos", &self.pos);
ff.finish()
}
@ -409,12 +395,6 @@ pub enum Expr {
),
/// xxx `.` method `(` expr `,` ... `)`
MethodCall(Box<FnCallExpr>, Position),
/// Stack slot for function calls. See [`FnCallExpr`] for more details.
///
/// This variant does not map to any language structure. It is used in function calls with
/// constant arguments where the `usize` number indexes into an array containing a list of
/// constant arguments for the function call.
Stack(usize, Position),
/// { [statement][Stmt] ... }
Stmt(Box<StmtBlock>),
/// func `(` expr `,` ... `)`
@ -490,7 +470,6 @@ impl fmt::Debug for Expr {
}
Self::Property(x, ..) => write!(f, "Property({})", x.2),
Self::MethodCall(x, ..) => f.debug_tuple("MethodCall").field(x).finish(),
Self::Stack(x, ..) => write!(f, "ConstantArg[{}]", x),
Self::Stmt(x) => {
let pos = x.span();
if !pos.is_none() {
@ -645,8 +624,7 @@ impl Expr {
namespace: None,
name: KEYWORD_FN_PTR.into(),
hashes: calc_fn_hash(f.fn_name(), 1).into(),
args: once(Self::Stack(0, pos)).collect(),
constants: once(f.fn_name().into()).collect(),
args: once(Self::StringConstant(f.fn_name().into(), pos)).collect(),
capture_parent_scope: false,
pos,
}
@ -704,7 +682,6 @@ impl Expr {
| Self::Array(.., pos)
| Self::Map(.., pos)
| Self::Variable(.., pos, _)
| Self::Stack(.., pos)
| Self::And(.., pos)
| Self::Or(.., pos)
| Self::Index(.., pos)
@ -759,7 +736,6 @@ impl Expr {
| Self::Dot(.., pos)
| Self::Index(.., pos)
| Self::Variable(.., pos, _)
| Self::Stack(.., pos)
| Self::FnCall(.., pos)
| Self::MethodCall(.., pos)
| Self::Custom(.., pos)
@ -786,7 +762,7 @@ impl Expr {
Self::Stmt(x) => x.iter().all(Stmt::is_pure),
Self::Variable(..) | Self::Stack(..) => true,
Self::Variable(..) => true,
_ => self.is_constant(),
}
@ -810,8 +786,7 @@ impl Expr {
| Self::IntegerConstant(..)
| Self::CharConstant(..)
| Self::StringConstant(..)
| Self::Unit(..)
| Self::Stack(..) => true,
| Self::Unit(..) => true,
Self::InterpolatedString(x, ..) | Self::Array(x, ..) => x.iter().all(Self::is_constant),
@ -872,8 +847,6 @@ impl Expr {
Token::LeftParen => true,
_ => false,
},
Self::Stack(..) => false,
}
}
/// Recursively walk this expression.

View File

@ -684,16 +684,13 @@ impl Engine {
Expr::MethodCall(x, ..)
if _parent_chain_type == ChainType::Dotting && !x.is_qualified() =>
{
let crate::ast::FnCallExpr {
args, constants, ..
} = x.as_ref();
let crate::ast::FnCallExpr { args, .. } = x.as_ref();
let (values, pos) = args.iter().try_fold(
(crate::FnArgsVec::with_capacity(args.len()), Position::NONE),
|(mut values, mut pos), expr| {
let (value, arg_pos) = self.get_arg_value(
scope, global, state, lib, this_ptr, expr, constants, level,
)?;
let (value, arg_pos) =
self.get_arg_value(scope, global, state, lib, this_ptr, expr, level)?;
if values.is_empty() {
pos = arg_pos;
}
@ -732,15 +729,13 @@ impl Engine {
Expr::MethodCall(x, ..)
if _parent_chain_type == ChainType::Dotting && !x.is_qualified() =>
{
let crate::ast::FnCallExpr {
args, constants, ..
} = x.as_ref();
let crate::ast::FnCallExpr { args, .. } = x.as_ref();
let (values, pos) = args.iter().try_fold(
(crate::FnArgsVec::with_capacity(args.len()), Position::NONE),
|(mut values, mut pos), expr| {
let (value, arg_pos) = self.get_arg_value(
scope, global, state, lib, this_ptr, expr, constants, level,
scope, global, state, lib, this_ptr, expr, level,
)?;
if values.is_empty() {
pos = arg_pos

View File

@ -212,7 +212,6 @@ impl Engine {
capture_parent_scope: capture,
hashes,
args,
constants,
..
} = expr;
@ -222,8 +221,7 @@ impl Engine {
let hash = hashes.native;
return self.make_qualified_function_call(
scope, global, state, lib, this_ptr, namespace, name, args, constants, hash, pos,
level,
scope, global, state, lib, this_ptr, namespace, name, args, hash, pos, level,
);
}
@ -234,8 +232,8 @@ impl Engine {
);
self.make_function_call(
scope, global, state, lib, this_ptr, name, first_arg, args, constants, *hashes,
*capture, pos, level,
scope, global, state, lib, this_ptr, name, first_arg, args, *hashes, *capture, pos,
level,
)
}

View File

@ -958,19 +958,17 @@ impl Engine {
lib: &[&Module],
this_ptr: &mut Option<&mut Dynamic>,
arg_expr: &Expr,
constants: &[Dynamic],
level: usize,
) -> RhaiResultOf<(Dynamic, Position)> {
Ok((
if let Expr::Stack(slot, ..) = arg_expr {
#[cfg(feature = "debugging")]
if self.debugger.is_some() {
if let Some(value) = arg_expr.get_literal_value() {
#[cfg(feature = "debugging")]
self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?;
constants[*slot].clone()
} else if let Some(value) = arg_expr.get_literal_value() {
#[cfg(feature = "debugging")]
self.run_debugger(scope, global, state, lib, this_ptr, arg_expr, level)?;
value
} else {
return Ok((value, arg_expr.start_position()));
}
}
// Do not match function exit for arguments
#[cfg(feature = "debugging")]
let reset_debugger = global.debugger.clear_status_if(|status| {
@ -983,10 +981,7 @@ impl Engine {
#[cfg(feature = "debugging")]
global.debugger.reset_status(reset_debugger);
result?
},
arg_expr.start_position(),
))
Ok((result?, arg_expr.start_position()))
}
/// Call a function in normal function-call style.
@ -1000,7 +995,6 @@ impl Engine {
fn_name: &str,
first_arg: Option<&Expr>,
args_expr: &[Expr],
constants: &[Dynamic],
hashes: FnCallHashes,
capture_scope: bool,
pos: Position,
@ -1019,7 +1013,7 @@ impl Engine {
KEYWORD_FN_PTR_CALL if total_args >= 1 => {
let arg = first_arg.unwrap();
let (arg_value, arg_pos) =
self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?;
self.get_arg_value(scope, global, state, lib, this_ptr, arg, level)?;
if !arg_value.is::<FnPtr>() {
return Err(self.make_type_mismatch_err::<FnPtr>(
@ -1054,7 +1048,7 @@ impl Engine {
KEYWORD_FN_PTR if total_args == 1 => {
let arg = first_arg.unwrap();
let (arg_value, arg_pos) =
self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?;
self.get_arg_value(scope, global, state, lib, this_ptr, arg, level)?;
// Fn - only in function call style
return arg_value
@ -1068,8 +1062,8 @@ impl Engine {
// Handle curry()
KEYWORD_FN_PTR_CURRY if total_args > 1 => {
let first = first_arg.unwrap();
let (arg_value, arg_pos) = self
.get_arg_value(scope, global, state, lib, this_ptr, first, constants, level)?;
let (arg_value, arg_pos) =
self.get_arg_value(scope, global, state, lib, this_ptr, first, level)?;
if !arg_value.is::<FnPtr>() {
return Err(self.make_type_mismatch_err::<FnPtr>(
@ -1082,9 +1076,8 @@ impl Engine {
// Append the new curried arguments to the existing list.
let fn_curry = a_expr.iter().try_fold(fn_curry, |mut curried, expr| {
let (value, ..) = self.get_arg_value(
scope, global, state, lib, this_ptr, expr, constants, level,
)?;
let (value, ..) =
self.get_arg_value(scope, global, state, lib, this_ptr, expr, level)?;
curried.push(value);
Ok::<_, RhaiError>(curried)
})?;
@ -1097,7 +1090,7 @@ impl Engine {
crate::engine::KEYWORD_IS_SHARED if total_args == 1 => {
let arg = first_arg.unwrap();
let (arg_value, ..) =
self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?;
self.get_arg_value(scope, global, state, lib, this_ptr, arg, level)?;
return Ok(arg_value.is_shared().into());
}
@ -1105,16 +1098,15 @@ impl Engine {
#[cfg(not(feature = "no_function"))]
crate::engine::KEYWORD_IS_DEF_FN if total_args == 2 => {
let first = first_arg.unwrap();
let (arg_value, arg_pos) = self
.get_arg_value(scope, global, state, lib, this_ptr, first, constants, level)?;
let (arg_value, arg_pos) =
self.get_arg_value(scope, global, state, lib, this_ptr, first, level)?;
let fn_name = arg_value
.into_immutable_string()
.map_err(|typ| self.make_type_mismatch_err::<ImmutableString>(typ, arg_pos))?;
let (arg_value, arg_pos) = self.get_arg_value(
scope, global, state, lib, this_ptr, &a_expr[0], constants, level,
)?;
let (arg_value, arg_pos) =
self.get_arg_value(scope, global, state, lib, this_ptr, &a_expr[0], level)?;
let num_params = arg_value
.as_int()
@ -1133,7 +1125,7 @@ impl Engine {
KEYWORD_IS_DEF_VAR if total_args == 1 => {
let arg = first_arg.unwrap();
let (arg_value, arg_pos) =
self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?;
self.get_arg_value(scope, global, state, lib, this_ptr, arg, level)?;
let var_name = arg_value
.into_immutable_string()
.map_err(|typ| self.make_type_mismatch_err::<ImmutableString>(typ, arg_pos))?;
@ -1146,7 +1138,7 @@ impl Engine {
let orig_scope_len = scope.len();
let arg = first_arg.unwrap();
let (arg_value, pos) =
self.get_arg_value(scope, global, state, lib, this_ptr, arg, constants, level)?;
self.get_arg_value(scope, global, state, lib, this_ptr, arg, level)?;
let script = &arg_value
.into_immutable_string()
.map_err(|typ| self.make_type_mismatch_err::<ImmutableString>(typ, pos))?;
@ -1195,7 +1187,7 @@ impl Engine {
.map(|&v| v)
.chain(a_expr.iter())
.try_for_each(|expr| {
self.get_arg_value(scope, global, state, lib, this_ptr, expr, constants, level)
self.get_arg_value(scope, global, state, lib, this_ptr, expr, level)
.map(|(value, ..)| arg_values.push(value.flatten()))
})?;
args.extend(curry.iter_mut());
@ -1227,7 +1219,7 @@ impl Engine {
// func(x, ...) -> x.func(...)
a_expr.iter().try_for_each(|expr| {
self.get_arg_value(scope, global, state, lib, this_ptr, expr, constants, level)
self.get_arg_value(scope, global, state, lib, this_ptr, expr, level)
.map(|(value, ..)| arg_values.push(value.flatten()))
})?;
@ -1262,9 +1254,7 @@ impl Engine {
.into_iter()
.chain(a_expr.iter())
.try_for_each(|expr| {
self.get_arg_value(
scope, global, state, lib, this_ptr, expr, constants, level,
)
self.get_arg_value(scope, global, state, lib, this_ptr, expr, level)
.map(|(value, ..)| arg_values.push(value.flatten()))
})?;
args.extend(curry.iter_mut());
@ -1290,7 +1280,6 @@ impl Engine {
namespace: &crate::module::Namespace,
fn_name: &str,
args_expr: &[Expr],
constants: &[Dynamic],
hash: u64,
pos: Position,
level: usize,
@ -1313,7 +1302,7 @@ impl Engine {
arg_values.push(Dynamic::UNIT);
args_expr.iter().skip(1).try_for_each(|expr| {
self.get_arg_value(scope, global, state, lib, this_ptr, expr, constants, level)
self.get_arg_value(scope, global, state, lib, this_ptr, expr, level)
.map(|(value, ..)| arg_values.push(value.flatten()))
})?;
@ -1344,7 +1333,7 @@ impl Engine {
} else {
// func(..., ...) or func(mod::x, ...)
args_expr.iter().try_for_each(|expr| {
self.get_arg_value(scope, global, state, lib, this_ptr, expr, constants, level)
self.get_arg_value(scope, global, state, lib, this_ptr, expr, level)
.map(|(value, ..)| arg_values.push(value.flatten()))
})?;
args.extend(arg_values.iter_mut());

View File

@ -438,15 +438,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
Expr::FnCall(ref mut x2, ..) => {
state.set_dirty();
x.0 = Some(OpAssignment::new_from_base(&x2.name));
let value = mem::take(&mut x2.args[1]);
if let Expr::Stack(slot, pos) = value {
x.1.rhs =
Expr::from_dynamic(mem::take(x2.constants.get_mut(slot).unwrap()), pos);
} else {
x.1.rhs = value;
}
x.1.rhs = mem::take(&mut x2.args[1]);
}
ref expr => unreachable!("Expr::FnCall expected but gets {:?}", expr),
}
@ -1063,7 +1055,6 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
&& x.args[0].is_constant()
=> {
let fn_name = match x.args[0] {
Expr::Stack(slot, ..) => x.constants[slot].clone(),
Expr::StringConstant(ref s, ..) => s.clone().into(),
_ => Dynamic::UNIT
};
@ -1088,11 +1079,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
&& x.args.iter().all(Expr::is_constant) // all arguments are constants
//&& !is_valid_identifier(x.name.chars()) // cannot be scripted
=> {
let arg_values = &mut x.args.iter().map(|e| match e {
Expr::Stack(slot, ..) => x.constants[*slot].clone(),
_ => e.get_literal_value().unwrap()
}).collect::<StaticVec<_>>();
let arg_values = &mut x.args.iter().map(|e| e.get_literal_value().unwrap()).collect::<StaticVec<_>>();
let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect();
match x.name.as_str() {
@ -1131,14 +1118,21 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
x.args.iter_mut().for_each(|a| optimize_expr(a, state, false));
// Move constant arguments
let constants = &mut x.constants;
x.args.iter_mut().for_each(|arg| {
if let Some(value) = arg.get_literal_value() {
for arg in x.args.iter_mut() {
match arg {
Expr::DynamicConstant(..) | Expr::Unit(..)
| Expr::StringConstant(..) | Expr::CharConstant(..)
| Expr::BoolConstant(..) | Expr::IntegerConstant(..) => (),
#[cfg(not(feature = "no_float"))]
Expr:: FloatConstant(..) => (),
_ => if let Some(value) = arg.get_literal_value() {
state.set_dirty();
constants.push(value);
*arg = Expr::Stack(constants.len()-1, arg.start_position());
*arg = Expr::DynamicConstant(value.into(), arg.start_position());
},
}
}
});
}
// Eagerly call functions
@ -1154,10 +1148,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
let has_script_fn = false;
if !has_script_fn {
let arg_values = &mut x.args.iter().map(|e| match e {
Expr::Stack(slot, ..) => x.constants[*slot].clone(),
_ => e.get_literal_value().unwrap()
}).collect::<StaticVec<_>>();
let arg_values = &mut x.args.iter().map(|e| e.get_literal_value().unwrap()).collect::<StaticVec<_>>();
let result = match x.name.as_str() {
KEYWORD_TYPE_OF if arg_values.len() == 1 => Some(state.engine.map_type_name(arg_values[0].type_name()).into()),
@ -1181,10 +1172,18 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) {
optimize_expr(arg, state, false);
// Move constant arguments
if let Some(value) = arg.get_literal_value() {
match arg {
Expr::DynamicConstant(..) | Expr::Unit(..)
| Expr::StringConstant(..) | Expr::CharConstant(..)
| Expr::BoolConstant(..) | Expr::IntegerConstant(..) => (),
#[cfg(not(feature = "no_float"))]
Expr:: FloatConstant(..) => (),
_ => if let Some(value) = arg.get_literal_value() {
state.set_dirty();
x.constants.push(value);
*arg = Expr::Stack(x.constants.len()-1, arg.start_position());
*arg = Expr::DynamicConstant(value.into(), arg.start_position());
},
}
},

View File

@ -521,7 +521,6 @@ impl Engine {
namespace,
hashes,
args,
constants: StaticVec::new_const(),
pos: settings.pos,
}
.into_fn_call_expr(settings.pos));
@ -586,7 +585,6 @@ impl Engine {
namespace,
hashes,
args,
constants: StaticVec::new_const(),
pos: settings.pos,
}
.into_fn_call_expr(settings.pos));