perf(value): Reduce allocations with Cow

Change `value::Object`'s key to a `Cow` to avoid allocations when a `&'static str` is used. BREAKING CHANGE: When inserting items into a `value::Object`, the value needs to be turned into a `Cow<str>`, usually with `.into()`. This carries forward to inserting values into a `Context`.
cobalt-org · Sep 5, 2018 · 7fd1e62 · 7fd1e62
1 parent f52f89e
commit 7fd1e62
Show file tree

Hide file tree

Showing 13 changed files with 43 additions and 36 deletions.
diff --git a/README.md b/README.md
@@ -29,7 +29,7 @@ let template = liquid::ParserBuilder::with_liquid()
     .parse("Liquid! {{num | minus: 2}}").unwrap();
 
 let mut globals = liquid::Object::new();
-globals.insert("num".to_owned(), liquid::Value::scalar(4f64));
+globals.insert("num".into(), liquid::Value::scalar(4f64));
 
 let output = template.render(&globals).unwrap();
 assert_eq!(output, "Liquid! 2".to_string());

diff --git a/src/interpreter/context.rs b/src/interpreter/context.rs
@@ -2,6 +2,7 @@ use std::collections::HashMap;
 
 use error::{Error, Result};
 use value::{Index, Object, Value};
+use std::borrow;
 
 use super::Argument;
 use super::{BoxedValueFilter, FilterValue};
@@ -167,8 +168,11 @@ impl Context {
     }
 
     /// Sets a value in the global context.
-    pub fn set_global_val(&mut self, name: &str, val: Value) -> Option<Value> {
-        self.globals.insert(name.to_owned(), val)
+    pub fn set_global_val<S>(&mut self, name: S, val: Value) -> Option<Value>
+    where
+        S: Into<borrow::Cow<'static, str>>,
+    {
+        self.globals.insert(name.into(), val)
     }
 
     /// Sets a value to the rendering context.
@@ -179,9 +183,12 @@ impl Context {
     /// Panics if there is no frame on the local values stack. Context
     /// instances are created with a top-level stack frame in place, so
     /// this should never happen in a well-formed program.
-    pub fn set_val(&mut self, name: &str, val: Value) -> Option<Value> {
+    pub fn set_val<S>(&mut self, name: S, val: Value) -> Option<Value>
+    where
+        S: Into<borrow::Cow<'static, str>>,
+    {
         match self.stack.last_mut() {
-            Some(frame) => frame.insert(name.to_owned(), val),
+            Some(frame) => frame.insert(name.into(), val),
             None => panic!("Cannot insert into an empty stack"),
         }
     }
@@ -202,7 +209,7 @@ mod test {
     fn get_val_failure() {
         let mut ctx = Context::new();
         let mut post = Object::new();
-        post.insert("number".to_owned(), Value::scalar(42f64));
+        post.insert("number".into(), Value::scalar(42f64));
         ctx.set_global_val("post", Value::Object(post));
         assert!(ctx.get_val("post.number").is_none());
     }
@@ -211,7 +218,7 @@ mod test {
     fn get_val_by_index() {
         let mut ctx = Context::new();
         let mut post = Object::new();
-        post.insert("number".to_owned(), Value::scalar(42f64));
+        post.insert("number".into(), Value::scalar(42f64));
         ctx.set_global_val("post", Value::Object(post));
         let indexes = vec![Index::with_key("post"), Index::with_key("number")];
         assert_eq!(

diff --git a/src/lib.rs b/src/lib.rs
@@ -14,7 +14,7 @@
 //!     .parse("Liquid! {{num | minus: 2}}").unwrap();
 //!
 //! let mut globals = liquid::Object::new();
-//! globals.insert("num".to_owned(), liquid::Value::scalar(4f64));
+//! globals.insert("num".into(), liquid::Value::scalar(4f64));
 //!
 //! let output = template.render(&globals).unwrap();
 //! assert_eq!(output, "Liquid! 2".to_string());

diff --git a/src/parser.rs b/src/parser.rs
@@ -259,7 +259,7 @@ impl Parser {
     ///     .parse_file("path/to/template.txt").unwrap();
     ///
     /// let mut globals = liquid::Object::new();
-    /// globals.insert("data".to_owned(), liquid::Value::scalar(4f64));
+    /// globals.insert("data".into(), liquid::Value::scalar(4f64));
     /// let output = template.render(&globals).unwrap();
     /// assert_eq!(output, "Liquid! 4\n".to_string());
     /// ```

diff --git a/src/tags/assign_tag.rs b/src/tags/assign_tag.rs
@@ -25,7 +25,7 @@ impl Renderable for Assign {
         let value = self.src
             .apply_filters(context)
             .trace_with(|| self.trace().into())?;
-        context.set_global_val(&self.dst, value);
+        context.set_global_val(self.dst.to_owned(), value);
         Ok(None)
     }
 }

diff --git a/src/tags/capture_block.rs b/src/tags/capture_block.rs
@@ -28,7 +28,7 @@ impl Renderable for Capture {
             .trace_with(|| self.trace().into())?
             .unwrap_or_else(|| "".to_owned());
 
-        context.set_global_val(&self.id, Value::scalar(output));
+        context.set_global_val(self.id.to_owned(), Value::scalar(output));
         Ok(None)
     }
 }

diff --git a/src/tags/for_block.rs b/src/tags/for_block.rs
@@ -131,22 +131,22 @@ impl Renderable for For {
                 let mut ret = String::default();
                 context.run_in_scope(|mut scope| {
                     let mut helper_vars = Object::new();
-                    helper_vars.insert("length".to_owned(), Value::scalar(range_len as i32));
+                    helper_vars.insert("length".into(), Value::scalar(range_len as i32));
 
                     for (i, v) in range.iter().enumerate() {
-                        helper_vars.insert("index0".to_owned(), Value::scalar(i as i32));
-                        helper_vars.insert("index".to_owned(), Value::scalar((i + 1) as i32));
+                        helper_vars.insert("index0".into(), Value::scalar(i as i32));
+                        helper_vars.insert("index".into(), Value::scalar((i + 1) as i32));
                         helper_vars.insert(
-                            "rindex0".to_owned(),
+                            "rindex0".into(),
                             Value::scalar((range_len - i - 1) as i32),
                         );
                         helper_vars
-                            .insert("rindex".to_owned(), Value::scalar((range_len - i) as i32));
-                        helper_vars.insert("first".to_owned(), Value::scalar(i == 0));
-                        helper_vars.insert("last".to_owned(), Value::scalar(i == (range_len - 1)));
+                            .insert("rindex".into(), Value::scalar((range_len - i) as i32));
+                        helper_vars.insert("first".into(), Value::scalar(i == 0));
+                        helper_vars.insert("last".into(), Value::scalar(i == (range_len - 1)));
 
                         scope.set_val("forloop", Value::Object(helper_vars.clone()));
-                        scope.set_val(&self.var_name, v.clone());
+                        scope.set_val(self.var_name.to_owned(), v.clone());
                         let inner = self.item_template
                             .render(&mut scope)
                             .trace_with(|| self.trace().into())

diff --git a/src/tags/if_block.rs b/src/tags/if_block.rs
@@ -502,7 +502,7 @@ mod test {
 
         let mut context = Context::new();
         let mut obj = Object::new();
-        obj.insert("Star Wars".to_owned(), Value::scalar("1977"));
+        obj.insert("Star Wars".into(), Value::scalar("1977"));
         context.set_global_val("movies", Value::Object(obj));
         let output = template.render(&mut context).unwrap();
         assert_eq!(output, Some("if true".to_owned()));

diff --git a/src/value/values.rs b/src/value/values.rs
@@ -32,7 +32,7 @@ pub enum Value {
 pub type Array = Vec<Value>;
 
 /// Type representing a Liquid object, payload of the `Value::Object` variant
-pub type Object = MapImpl<String, Value>;
+pub type Object = MapImpl<borrow::Cow<'static, str>, Value>;
 
 impl Value {
     pub fn scalar<T: Into<Scalar>>(value: T) -> Self {
@@ -57,7 +57,7 @@ impl Value {
             }
             Value::Object(ref x) => {
                 let arr: Vec<String> = x.iter()
-                    .map(|(k, v)| k.clone() + ": " + &v.to_string())
+                    .map(|(k, v)| format!("{}: {}", k, v))
                     .collect();
                 borrow::Cow::Owned(arr.join(", "))
             }
@@ -297,17 +297,17 @@ mod test {
     #[test]
     fn object_equality() {
         let a: Object = [
-            ("alpha".to_owned(), Value::scalar("1")),
-            ("beta".to_owned(), Value::scalar(2f64)),
+            ("alpha".into(), Value::scalar("1")),
+            ("beta".into(), Value::scalar(2f64)),
         ].into_iter()
             .cloned()
             .collect();
         let a = Value::Object(a);
 
         let b: Object = [
-            ("alpha".to_owned(), Value::scalar("1")),
-            ("beta".to_owned(), Value::scalar(2f64)),
-            ("gamma".to_owned(), Value::Array(vec![])),
+            ("alpha".into(), Value::scalar("1")),
+            ("beta".into(), Value::scalar(2f64)),
+            ("gamma".into(), Value::Array(vec![])),
         ].into_iter()
             .cloned()
             .collect();

diff --git a/tests/filters.rs b/tests/filters.rs
@@ -464,7 +464,7 @@ pub fn escape() {
     ];
     for t in &samples {
         let mut globals = liquid::Object::new();
-        globals.insert("var".to_owned(), liquid::Value::scalar(t.0));
+        globals.insert("var".into(), liquid::Value::scalar(t.0));
         let template = liquid::ParserBuilder::with_liquid()
             .build()
             .parse(text)
@@ -487,7 +487,7 @@ pub fn escape_once() {
     ];
     for t in &samples {
         let mut globals = liquid::Object::new();
-        globals.insert("var".to_owned(), liquid::Value::scalar(t.0));
+        globals.insert("var".into(), liquid::Value::scalar(t.0));
         let template = liquid::ParserBuilder::with_liquid()
             .build()
             .parse(text)

diff --git a/tests/fixtures.rs b/tests/fixtures.rs
@@ -53,8 +53,8 @@ numTwo: 6
 #[test]
 pub fn include() {
     let mut globals: liquid::Object = Default::default();
-    globals.insert("num".to_owned(), Value::scalar(5f64));
-    globals.insert("numTwo".to_owned(), Value::scalar(10f64));
+    globals.insert("num".into(), Value::scalar(5f64));
+    globals.insert("numTwo".into(), Value::scalar(10f64));
     compare_by_file("include", &globals);
 }
 

diff --git a/tests/parse_file.rs b/tests/parse_file.rs
@@ -66,8 +66,8 @@ numTwo: 6
 #[test]
 pub fn include_by_file() {
     let mut globals: Object = Default::default();
-    globals.insert("num".to_owned(), Value::scalar(5f64));
-    globals.insert("numTwo".to_owned(), Value::scalar(10f64));
+    globals.insert("num".into(), Value::scalar(5f64));
+    globals.insert("numTwo".into(), Value::scalar(10f64));
     compare_by_file("include", &globals);
 }
 

diff --git a/tests/value.rs b/tests/value.rs
@@ -143,9 +143,9 @@ pub fn deserialize_object() {
     let actual: liquid::Value =
         serde_yaml::from_str("---\nNum: 1\nBool: true\nStr: \"true\"").unwrap();
     let expected: liquid::Object = [
-        ("Num".to_owned(), liquid::Value::scalar(1f64)),
-        ("Bool".to_owned(), liquid::Value::scalar(true)),
-        ("Str".to_owned(), liquid::Value::scalar("true")),
+        ("Num".into(), liquid::Value::scalar(1f64)),
+        ("Bool".into(), liquid::Value::scalar(true)),
+        ("Str".into(), liquid::Value::scalar("true")),
     ].iter()
         .cloned()
         .collect();