Improve speed on common dot/index expressions.

This commit is contained in:
Stephen Chung 2022-06-07 20:38:05 +08:00
parent 84e3296559
commit 8501d9d33f
5 changed files with 51 additions and 26 deletions

View File

@ -401,6 +401,10 @@ pub enum Expr {
/// func `(` expr `,` ... `)` /// func `(` expr `,` ... `)`
FnCall(Box<FnCallExpr>, Position), FnCall(Box<FnCallExpr>, Position),
/// lhs `.` rhs /// lhs `.` rhs
///
/// ### Flags
///
/// No flags are defined at this time. Use [`NONE`][ASTFlags::NONE].
Dot(Box<BinaryExpr>, ASTFlags, Position), Dot(Box<BinaryExpr>, ASTFlags, Position),
/// lhs `[` rhs `]` /// lhs `[` rhs `]`
/// ///

View File

@ -118,7 +118,7 @@ impl ChainArgument {
impl Engine { impl Engine {
/// Chain-evaluate a dot/index chain. /// Chain-evaluate a dot/index chain.
/// [`Position`] in [`EvalAltResult`] is [`NONE`][Position::NONE] and must be set afterwards. /// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards.
fn eval_dot_index_chain_helper( fn eval_dot_index_chain_helper(
&self, &self,
global: &mut GlobalRuntimeState, global: &mut GlobalRuntimeState,
@ -620,11 +620,41 @@ impl Engine {
let idx_values = &mut StaticVec::new_const(); let idx_values = &mut StaticVec::new_const();
self.eval_dot_index_chain_arguments( match rhs {
scope, global, caches, lib, this_ptr, rhs, options, chain_type, idx_values, 0, level, // Short-circuit for simple property access: {expr}.prop
)?; Expr::Property(..) if chain_type == ChainType::Dotting => {
idx_values.push(ChainArgument::Property(rhs.position()))
}
Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"),
// Short-circuit for simple method call: {expr}.func()
Expr::FnCall(x, ..) if chain_type == ChainType::Dotting && x.args.is_empty() => {
idx_values.push(ChainArgument::MethodCallArgs(
Default::default(),
Position::NONE,
))
}
// Short-circuit for method call with all literal arguments: {expr}.func(1, 2, 3)
Expr::FnCall(x, ..)
if chain_type == ChainType::Dotting && x.args.iter().all(Expr::is_constant) =>
{
let args: Vec<_> = x
.args
.iter()
.map(|expr| expr.get_literal_value().unwrap())
.collect();
let is_assignment = new_val.is_some(); idx_values.push(ChainArgument::MethodCallArgs(
args.into_boxed_slice(),
x.args[0].position(),
))
}
_ => {
self.eval_dot_index_chain_arguments(
scope, global, caches, lib, this_ptr, rhs, options, chain_type, idx_values, 0,
level,
)?;
}
}
match lhs { match lhs {
// id.??? or id[???] // id.??? or id[???]
@ -645,11 +675,9 @@ impl Engine {
global, caches, lib, &mut None, obj_ptr, root, expr, rhs, options, idx_values, global, caches, lib, &mut None, obj_ptr, root, expr, rhs, options, idx_values,
chain_type, level, new_val, chain_type, level, new_val,
) )
.map(|(v, ..)| v)
.map_err(|err| err.fill_position(op_pos))
} }
// {expr}.??? = ??? or {expr}[???] = ??? // {expr}.??? = ??? or {expr}[???] = ???
_ if is_assignment => unreachable!("cannot assign to an expression"), _ if new_val.is_some() => unreachable!("cannot assign to an expression"),
// {expr}.??? or {expr}[???] // {expr}.??? or {expr}[???]
expr => { expr => {
let value = self let value = self
@ -657,15 +685,16 @@ impl Engine {
.flatten(); .flatten();
let obj_ptr = &mut value.into(); let obj_ptr = &mut value.into();
let root = ("", expr.start_position()); let root = ("", expr.start_position());
self.eval_dot_index_chain_helper( self.eval_dot_index_chain_helper(
global, caches, lib, this_ptr, obj_ptr, root, expr, rhs, options, idx_values, global, caches, lib, this_ptr, obj_ptr, root, expr, rhs, options, idx_values,
chain_type, level, new_val, chain_type, level, new_val,
) )
.map(|(v, ..)| if is_assignment { Dynamic::UNIT } else { v }) }
}
.map(|(v, ..)| v)
.map_err(|err| err.fill_position(op_pos)) .map_err(|err| err.fill_position(op_pos))
} }
}
}
/// Evaluate a chain of indexes and store the results in a [`StaticVec`]. /// Evaluate a chain of indexes and store the results in a [`StaticVec`].
/// [`StaticVec`] is used to avoid an allocation in the overwhelming cases of /// [`StaticVec`] is used to avoid an allocation in the overwhelming cases of
@ -798,7 +827,6 @@ impl Engine {
} }
/// Call a get indexer. /// Call a get indexer.
/// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards.
#[inline(always)] #[inline(always)]
fn call_indexer_get( fn call_indexer_get(
&self, &self,
@ -822,7 +850,6 @@ impl Engine {
} }
/// Call a set indexer. /// Call a set indexer.
/// [`Position`] in [`EvalAltResult`] may be [`NONE`][Position::NONE] and should be set afterwards.
#[inline(always)] #[inline(always)]
fn call_indexer_set( fn call_indexer_set(
&self, &self,

View File

@ -184,9 +184,7 @@ impl Engine {
*target.as_mut() = new_val; *target.as_mut() = new_val;
} }
target target.propagate_changed_value(op_info.pos)
.propagate_changed_value()
.map_err(|err| err.fill_position(op_info.pos))
} }
/// Evaluate a statement. /// Evaluate a statement.

View File

@ -1,7 +1,7 @@
//! Type to hold a mutable reference to the target of an evaluation. //! Type to hold a mutable reference to the target of an evaluation.
use crate::types::dynamic::Variant; use crate::types::dynamic::Variant;
use crate::{Dynamic, RhaiResultOf}; use crate::{Dynamic, Position, RhaiResultOf};
use std::ops::{Deref, DerefMut}; use std::ops::{Deref, DerefMut};
#[cfg(feature = "no_std")] #[cfg(feature = "no_std")]
use std::prelude::v1::*; use std::prelude::v1::*;
@ -272,10 +272,8 @@ impl<'a> Target<'a> {
} }
/// Propagate a changed value back to the original source. /// Propagate a changed value back to the original source.
/// This has no effect for direct references. /// This has no effect for direct references.
///
/// [`Position`] in [`EvalAltResult`] is [`NONE`][Position::NONE] and should be set afterwards.
#[inline] #[inline]
pub fn propagate_changed_value(&mut self) -> RhaiResultOf<()> { pub fn propagate_changed_value(&mut self, pos: Position) -> RhaiResultOf<()> {
match self { match self {
Self::RefMut(..) | Self::TempValue(..) => (), Self::RefMut(..) | Self::TempValue(..) => (),
#[cfg(not(feature = "no_closure"))] #[cfg(not(feature = "no_closure"))]
@ -287,7 +285,7 @@ impl<'a> Target<'a> {
Box::new(crate::ERR::ErrorMismatchDataType( Box::new(crate::ERR::ErrorMismatchDataType(
"bool".to_string(), "bool".to_string(),
err.to_string(), err.to_string(),
crate::Position::NONE, pos,
)) ))
})?; })?;
@ -317,7 +315,7 @@ impl<'a> Target<'a> {
Box::new(crate::ERR::ErrorMismatchDataType( Box::new(crate::ERR::ErrorMismatchDataType(
"integer".to_string(), "integer".to_string(),
err.to_string(), err.to_string(),
crate::Position::NONE, pos,
)) ))
})?; })?;
@ -338,7 +336,7 @@ impl<'a> Target<'a> {
Box::new(crate::ERR::ErrorMismatchDataType( Box::new(crate::ERR::ErrorMismatchDataType(
"INT".to_string(), "INT".to_string(),
err.to_string(), err.to_string(),
crate::Position::NONE, pos,
)) ))
})?; })?;
@ -363,7 +361,7 @@ impl<'a> Target<'a> {
Box::new(crate::ERR::ErrorMismatchDataType( Box::new(crate::ERR::ErrorMismatchDataType(
"char".to_string(), "char".to_string(),
err.to_string(), err.to_string(),
crate::Position::NONE, pos,
)) ))
})?; })?;

View File

@ -920,9 +920,7 @@ impl Engine {
// Propagate the changed value back to the source if necessary // Propagate the changed value back to the source if necessary
if updated { if updated {
target target.propagate_changed_value(pos)?;
.propagate_changed_value()
.map_err(|err| err.fill_position(pos))?;
} }
Ok((result, updated)) Ok((result, updated))