perf: Speed up variable accesses

This leverages the `Cow` in `Scalar` to avoid allocations for creating a variable `Path`. BREAKING CHANGE: Variable/expression evaluations now return references.
cobalt-org · Nov 19, 2018 · f739248 · f739248
1 parent 08e18fc
commit f739248
Show file tree

Hide file tree

Showing 12 changed files with 185 additions and 162 deletions.
diff --git a/liquid-compiler/src/token.rs b/liquid-compiler/src/token.rs
@@ -131,7 +131,7 @@ mod test {
         let ctx = Context::new();
         let t = Token::StringLiteral("hello".to_owned());
         assert_eq!(
-            t.to_arg().unwrap().evaluate(&ctx).unwrap(),
+            *t.to_arg().unwrap().evaluate(&ctx).unwrap(),
             Value::scalar("hello")
         );
     }
@@ -140,7 +140,7 @@ mod test {
     fn evaluate_handles_number_literals() {
         let ctx = Context::new();
         assert_eq!(
-            Token::FloatLiteral(42f64)
+            *Token::FloatLiteral(42f64)
                 .to_arg()
                 .unwrap()
                 .evaluate(&ctx)
@@ -150,7 +150,7 @@ mod test {
 
         let ctx = Context::new();
         assert_eq!(
-            Token::IntegerLiteral(42i32)
+            *Token::IntegerLiteral(42i32)
                 .to_arg()
                 .unwrap()
                 .evaluate(&ctx)
@@ -163,7 +163,7 @@ mod test {
     fn evaluate_handles_boolean_literals() {
         let ctx = Context::new();
         assert_eq!(
-            Token::BooleanLiteral(true)
+            *Token::BooleanLiteral(true)
                 .to_arg()
                 .unwrap()
                 .evaluate(&ctx)
@@ -172,7 +172,7 @@ mod test {
         );
 
         assert_eq!(
-            Token::BooleanLiteral(false)
+            *Token::BooleanLiteral(false)
                 .to_arg()
                 .unwrap()
                 .evaluate(&ctx)
@@ -186,7 +186,7 @@ mod test {
         let mut ctx = Context::new();
         ctx.stack_mut().set_global("var0", Value::scalar(42f64));
         assert_eq!(
-            Token::Identifier("var0".to_owned())
+            *Token::Identifier("var0".to_owned())
                 .to_arg()
                 .unwrap()
                 .evaluate(&ctx)

diff --git a/liquid-interpreter/src/context.rs b/liquid-interpreter/src/context.rs
@@ -84,7 +84,7 @@ where
 
 impl<'a, 'g> CycleState<'a, 'g> {
     /// See `cycle` tag.
-    pub fn cycle_element(&mut self, name: &str, values: &[Expression]) -> Result<Value> {
+    pub fn cycle_element<'c>(&'c mut self, name: &str, values: &'c [Expression]) -> Result<&'c Value> {
         let index = self.context.cycles.cycle_index(name, values.len());
         if index >= values.len() {
             return Err(Error::with_msg(

diff --git a/liquid-interpreter/src/expression.rs b/liquid-interpreter/src/expression.rs
@@ -39,24 +39,24 @@ impl Expression {
     }
 
     /// Convert to a `Value`.
-    pub fn try_evaluate(&self, context: &Context) -> Option<Value> {
+    pub fn try_evaluate<'c>(&'c self, context: &'c Context) -> Option<&'c Value> {
         let val = match *self {
-            Expression::Literal(ref x) => x.clone(),
+            Expression::Literal(ref x) => &x,
             Expression::Variable(ref x) => {
                 let path = x.try_evaluate(context)?;
-                context.stack().try_get(&path)?.clone()
+                context.stack().try_get(&path)?
             }
         };
         Some(val)
     }
 
     /// Convert to a `Value`.
-    pub fn evaluate(&self, context: &Context) -> Result<Value> {
+    pub fn evaluate<'c>(&'c self, context: &'c Context) -> Result<&'c Value> {
         let val = match *self {
-            Expression::Literal(ref x) => x.clone(),
+            Expression::Literal(ref x) => x,
             Expression::Variable(ref x) => {
                 let path = x.evaluate(context)?;
-                context.stack().get(&path)?.clone()
+                context.stack().get(&path)?
             }
         };
         Ok(val)

diff --git a/liquid-interpreter/src/filter_chain.rs b/liquid-interpreter/src/filter_chain.rs
@@ -54,7 +54,7 @@ impl FilterChain {
     /// Process `Value` expression within `context`'s stack.
     pub fn evaluate(&self, context: &Context) -> Result<Value> {
         // take either the provided value or the value from the provided variable
-        let mut entry = self.entry.evaluate(context)?;
+        let mut entry = self.entry.evaluate(context)?.to_owned();
 
         // apply all specified filters
         for filter in &self.filters {
@@ -65,7 +65,7 @@ impl FilterChain {
             let arguments: Result<Vec<Value>> = filter
                 .arguments
                 .iter()
-                .map(|a| a.evaluate(context))
+                .map(|a| Ok(a.evaluate(context)?.to_owned()))
                 .collect();
             let arguments = arguments?;
             entry = f

diff --git a/liquid-interpreter/src/variable.rs b/liquid-interpreter/src/variable.rs
@@ -29,26 +29,26 @@ impl Variable {
     }
 
     /// Convert to a `Path`.
-    pub fn try_evaluate(&self, context: &Context) -> Option<Path> {
+    pub fn try_evaluate<'c>(&'c self, context: &'c Context) -> Option<Path<'c>> {
         let mut path = Path::with_index(self.variable.clone());
         path.reserve(self.indexes.len());
         for expr in &self.indexes {
             let v = expr.try_evaluate(context)?;
-            let s = v.into_scalar()?;
+            let s = v.as_scalar()?.as_ref();
             path.push(s);
         }
         Some(path)
     }
 
     /// Convert to a `Path`.
-    pub fn evaluate(&self, context: &Context) -> Result<Path> {
-        let mut path = Path::with_index(self.variable.clone());
+    pub fn evaluate<'c>(&'c self, context: &'c Context) -> Result<Path<'c>> {
+        let mut path = Path::with_index(self.variable.as_ref());
         path.reserve(self.indexes.len());
         for expr in &self.indexes {
             let v = expr.evaluate(context)?;
             let s = v.as_scalar()
                 .ok_or_else(|| Error::with_msg(format!("Expected scalar, found `{}`", v)))?
-                .to_owned();
+                .as_ref();
             path.push(s);
         }
         Ok(path)

diff --git a/liquid-value/src/path.rs b/liquid-value/src/path.rs
@@ -3,23 +3,23 @@ use std::slice;
 
 use itertools;
 
-use super::Scalar;
+use super::ScalarCow;
 
 /// Path to a value in an `Object`.
 ///
 /// There is guaranteed always at least one element.
 #[derive(Clone, Debug, PartialEq)]
-pub struct Path(Vec<Scalar>);
+pub struct Path<'s>(Vec<ScalarCow<'s>>);
 
-impl Path {
+impl<'s> Path<'s> {
     /// Create a `Value` reference.
-    pub fn with_index<I: Into<Scalar>>(value: I) -> Self {
+    pub fn with_index<I: Into<ScalarCow<'s>>>(value: I) -> Self {
         let indexes = vec![value.into()];
         Path(indexes)
     }
 
     /// Append an index.
-    pub fn push<I: Into<Scalar>>(&mut self, value: I) {
+    pub fn push<I: Into<ScalarCow<'s>>>(&mut self, value: I) {
         self.0.push(value.into());
     }
 
@@ -39,41 +39,41 @@ impl Path {
 
     /// Extracts a slice containing the entire vector.
     #[inline]
-    pub fn as_slice(&self) -> &[Scalar] {
+    pub fn as_slice(&self) -> &[ScalarCow<'s>] {
         self.0.as_slice()
     }
 }
 
-impl Extend<Scalar> for Path {
-    fn extend<T: IntoIterator<Item = Scalar>>(&mut self, iter: T) {
+impl<'s> Extend<ScalarCow<'s>> for Path<'s> {
+    fn extend<T: IntoIterator<Item = ScalarCow<'s>>>(&mut self, iter: T) {
         self.0.extend(iter);
     }
 }
 
-impl ::std::ops::Deref for Path {
-    type Target = [Scalar];
+impl<'s> ::std::ops::Deref for Path<'s> {
+    type Target = [ScalarCow<'s>];
 
     #[inline]
     fn deref( &self ) -> &Self::Target {
         &self.0
     }
 }
 
-impl ::std::borrow::Borrow<[Scalar]> for Path {
+impl<'s> ::std::borrow::Borrow<[ScalarCow<'s>]> for Path<'s> {
     #[inline]
-    fn borrow(&self) -> &[Scalar] {
+    fn borrow(&self) -> &[ScalarCow<'s>] {
         self
     }
 }
 
-impl AsRef<[Scalar]> for Path {
+impl<'s> AsRef<[ScalarCow<'s>]> for Path<'s> {
     #[inline]
-    fn as_ref(&self) -> &[Scalar] {
+    fn as_ref(&self) -> &[ScalarCow<'s>] {
         self
     }
 }
 
-impl fmt::Display for Path {
+impl<'s> fmt::Display for Path<'s> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let data = itertools::join(self.iter(), ".");
         write!(f, "{}", data)
@@ -82,13 +82,13 @@ impl fmt::Display for Path {
 
 /// Iterate over indexes in a `Value`'s `Path`.
 #[derive(Debug)]
-pub struct PathIter<'i>(slice::Iter<'i, Scalar>);
+pub struct PathIter<'i, 's: 'i>(slice::Iter<'i, ScalarCow<'s>>);
 
-impl<'i> Iterator for PathIter<'i> {
-    type Item = &'i Scalar;
+impl<'i, 's: 'i> Iterator for PathIter<'i, 's> {
+    type Item = &'i ScalarCow<'s>;
 
     #[inline]
-    fn next(&mut self) -> Option<&'i Scalar> {
+    fn next(&mut self) -> Option<&'i ScalarCow<'s>> {
         self.0.next()
     }
 
@@ -103,12 +103,12 @@ impl<'i> Iterator for PathIter<'i> {
     }
 }
 
-impl<'i> ExactSizeIterator for PathIter<'i>{
+impl<'i, 's: 'i> ExactSizeIterator for PathIter<'i, 's>{
     #[inline]
     fn len(&self) -> usize {
         self.0.len()
     }
 }
 
 /// Path to a value in an `Object`.
-pub type PathRef<'s> = &'s [Scalar];
+pub type PathRef<'p, 's> = &'p [ScalarCow<'s>];