perf: Reduce string cloning

This change attempts to strike a balance between what parts of the liquid API need to be dynamic and static. This makes the assumption that all block, tag, and filter names are known statically and uses a `&'static str` to represent them. Old ``` running 6 tests test bench_parse_template ... bench: 24,715 ns/iter (+/- 1,966) test bench_parse_text ... bench: 2,610 ns/iter (+/- 248) test bench_parse_variable ... bench: 4,628 ns/iter (+/- 425) test bench_render_template ... bench: 9,049 ns/iter (+/- 598) test bench_render_text ... bench: 2,204 ns/iter (+/- 151) test bench_render_variable ... bench: 6,867 ns/iter (+/- 437) ``` New ``` running 6 tests test bench_parse_template ... bench: 22,625 ns/iter (+/- 1,986) test bench_parse_text ... bench: 826 ns/iter (+/- 58) test bench_parse_variable ... bench: 2,871 ns/iter (+/- 175) test bench_render_template ... bench: 6,938 ns/iter (+/- 614) test bench_render_text ... bench: 432 ns/iter (+/- 29) test bench_render_variable ... bench: 6,201 ns/iter (+/- 537) ``` In the long term, we want to stop cloning `LiquidOptions` on every parse and `globals` on every render. This change reduced the impact of the former. BREAKING CHANGE: The API now takes `&'static str` instead of `String` for block, tag, and filter names
cobalt-org · Jan 14, 2018 · 3d93928 · 3d93928
1 parent c0eadd5
commit 3d93928
Show file tree

Hide file tree

Showing 15 changed files with 83 additions and 60 deletions.
diff --git a/src/compiler/options.rs b/src/compiler/options.rs
@@ -7,8 +7,8 @@ use super::NullInclude;
 
 #[derive(Clone)]
 pub struct LiquidOptions {
-    pub blocks: HashMap<String, BoxedBlockParser>,
-    pub tags: HashMap<String, BoxedTagParser>,
+    pub blocks: HashMap<&'static str, BoxedBlockParser>,
+    pub tags: HashMap<&'static str, BoxedTagParser>,
     pub include_source: Box<Include>,
 }
 

diff --git a/src/compiler/parser.rs b/src/compiler/parser.rs
@@ -53,8 +53,8 @@ fn parse_expression(tokens: &[Token], options: &LiquidOptions) -> Result<Box<Ren
             result.extend(indexes);
             Ok(Box::new(result))
         }
-        Some(&Token::Identifier(ref x)) if options.tags.contains_key(x) => {
-            options.tags[x].parse(x, &tokens[1..], options)
+        Some(&Token::Identifier(ref x)) if options.tags.contains_key(x.as_str()) => {
+            options.tags[x.as_str()].parse(x, &tokens[1..], options)
         }
         None => Error::parser("expression", None),
         _ => {
@@ -169,12 +169,12 @@ fn parse_tag(iter: &mut Iter<Element>,
     let tag = &tokens[0];
     match *tag {
         // is a tag
-        Token::Identifier(ref x) if options.tags.contains_key(x) => {
-            options.tags[x].parse(x, &tokens[1..], options)
+        Token::Identifier(ref x) if options.tags.contains_key(x.as_str()) => {
+            options.tags[x.as_str()].parse(x, &tokens[1..], options)
         }
 
         // is a block
-        Token::Identifier(ref x) if options.blocks.contains_key(x) => {
+        Token::Identifier(ref x) if options.blocks.contains_key(x.as_str()) => {
             // Collect all the inner elements of this block until we find a
             // matching "end<blockname>" tag. Note that there may be nested blocks
             // of the same type (and hence have the same closing delimiter) *inside*
@@ -203,7 +203,7 @@ fn parse_tag(iter: &mut Iter<Element>,
                 };
                 children.push(t.clone())
             }
-            options.blocks[x].parse(x, &tokens[1..], &children, options)
+            options.blocks[x.as_str()].parse(x, &tokens[1..], &children, options)
         }
 
         ref x => Err(Error::Parser(format!("parse_tag: {:?} not implemented", x))),
@@ -270,7 +270,7 @@ pub fn split_block<'a>(tokens: &'a [Element],
     for (i, t) in tokens.iter().enumerate() {
         if let Element::Tag(ref args, _) = *t {
             match args[0] {
-                Token::Identifier(ref name) if options.blocks.contains_key(name) => {
+                Token::Identifier(ref name) if options.blocks.contains_key(name.as_str()) => {
                     stack.push("end".to_owned() + name);
                 }
 
@@ -391,9 +391,10 @@ mod test_split_block {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        let blocks: HashMap<String, BoxedBlockParser> = ["comment", "for", "if"]
+        let blocks: [&'static str; 3] = ["comment", "for", "if"];
+        let blocks: HashMap<&'static str, BoxedBlockParser> = blocks
             .into_iter()
-            .map(|name| (name.to_string(), (null_block as FnParseBlock).into()))
+            .map(|name| (*name, (null_block as FnParseBlock).into()))
             .collect();
         options.blocks = blocks;
         options

diff --git a/src/interpreter/context.rs b/src/interpreter/context.rs
@@ -29,7 +29,7 @@ pub struct Context {
     cycles: HashMap<String, usize>,
 
     // Public for backwards compatability
-    filters: HashMap<String, BoxedValueFilter>,
+    filters: HashMap<&'static str, BoxedValueFilter>,
 }
 
 impl Context {
@@ -43,7 +43,7 @@ impl Context {
         self
     }
 
-    pub fn with_filters(mut self, filters: HashMap<String, BoxedValueFilter>) -> Self {
+    pub fn with_filters(mut self, filters: HashMap<&'static str, BoxedValueFilter>) -> Self {
         self.filters = filters;
         self
     }
@@ -66,8 +66,8 @@ impl Context {
         Ok(Some(val))
     }
 
-    pub fn add_filter(&mut self, name: &str, filter: BoxedValueFilter) {
-        self.filters.insert(name.to_owned(), filter);
+    pub fn add_filter(&mut self, name: &'static str, filter: BoxedValueFilter) {
+        self.filters.insert(name, filter);
     }
 
     pub fn get_filter<'b>(&'b self, name: &str) -> Option<&'b FilterValue> {

diff --git a/src/parser.rs b/src/parser.rs
@@ -12,9 +12,9 @@ use super::Template;
 
 #[derive(Default, Clone)]
 pub struct ParserBuilder {
-    blocks: HashMap<String, compiler::BoxedBlockParser>,
-    tags: HashMap<String, compiler::BoxedTagParser>,
-    filters: HashMap<String, interpreter::BoxedValueFilter>,
+    blocks: HashMap<&'static str, compiler::BoxedBlockParser>,
+    tags: HashMap<&'static str, compiler::BoxedTagParser>,
+    filters: HashMap<&'static str, interpreter::BoxedValueFilter>,
     include_source: Option<Box<compiler::Include>>,
 }
 
@@ -129,20 +129,26 @@ impl ParserBuilder {
     }
 
     /// Inserts a new custom block into the parser
-    pub fn block<B: Into<compiler::BoxedBlockParser>>(mut self, name: &str, block: B) -> Self {
-        self.blocks.insert(name.to_owned(), block.into());
+    pub fn block<B: Into<compiler::BoxedBlockParser>>(mut self,
+                                                      name: &'static str,
+                                                      block: B)
+                                                      -> Self {
+        self.blocks.insert(name, block.into());
         self
     }
 
     /// Inserts a new custom tag into the parser
-    pub fn tag<T: Into<compiler::BoxedTagParser>>(mut self, name: &str, tag: T) -> Self {
-        self.tags.insert(name.to_owned(), tag.into());
+    pub fn tag<T: Into<compiler::BoxedTagParser>>(mut self, name: &'static str, tag: T) -> Self {
+        self.tags.insert(name, tag.into());
         self
     }
 
     /// Inserts a new custom filter into the parser
-    pub fn filter<F: Into<interpreter::BoxedValueFilter>>(mut self, name: &str, filter: F) -> Self {
-        self.filters.insert(name.to_owned(), filter.into());
+    pub fn filter<F: Into<interpreter::BoxedValueFilter>>(mut self,
+                                                          name: &'static str,
+                                                          filter: F)
+                                                          -> Self {
+        self.filters.insert(name, filter.into());
         self
     }
 
@@ -175,7 +181,7 @@ impl ParserBuilder {
 #[derive(Default, Clone)]
 pub struct Parser {
     options: compiler::LiquidOptions,
-    filters: HashMap<String, interpreter::BoxedValueFilter>,
+    filters: HashMap<&'static str, interpreter::BoxedValueFilter>,
 }
 
 impl Parser {

diff --git a/src/tags/assign_tag.rs b/src/tags/assign_tag.rs
@@ -48,12 +48,15 @@ mod test {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        options.tags.insert("assign".to_owned(),
-                            (assign_tag as compiler::FnParseTag).into());
-        options.blocks.insert("if".to_owned(),
-                              (tags::if_block as compiler::FnParseBlock).into());
-        options.blocks.insert("for".to_owned(),
-                              (tags::for_block as compiler::FnParseBlock).into());
+        options
+            .tags
+            .insert("assign", (assign_tag as compiler::FnParseTag).into());
+        options
+            .blocks
+            .insert("if", (tags::if_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("for", (tags::for_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/tags/capture_block.rs b/src/tags/capture_block.rs
@@ -60,8 +60,9 @@ mod test {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        options.blocks.insert("capture".to_owned(),
-                              (capture_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("capture", (capture_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/tags/case_block.rs b/src/tags/case_block.rs
@@ -147,8 +147,9 @@ mod test {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        options.blocks.insert("case".to_owned(),
-                              (case_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("case", (case_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/tags/comment_block.rs b/src/tags/comment_block.rs
@@ -30,8 +30,9 @@ mod test {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        options.blocks.insert("comment".to_owned(),
-                              (comment_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("comment", (comment_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/tags/cycle_tag.rs b/src/tags/cycle_tag.rs
@@ -84,8 +84,9 @@ mod test {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        options.tags.insert("cycle".to_owned(),
-                            (cycle_tag as compiler::FnParseTag).into());
+        options
+            .tags
+            .insert("cycle", (cycle_tag as compiler::FnParseTag).into());
         options
     }
 

diff --git a/src/tags/for_block.rs b/src/tags/for_block.rs
@@ -234,8 +234,9 @@ mod test {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        options.blocks.insert("for".to_owned(),
-                              (for_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("for", (for_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/tags/if_block.rs b/src/tags/if_block.rs
@@ -165,9 +165,10 @@ mod test {
         let mut options = LiquidOptions::default();
         options
             .blocks
-            .insert("if".to_owned(), (if_block as compiler::FnParseBlock).into());
-        options.blocks.insert("unless".to_owned(),
-                              (unless_block as compiler::FnParseBlock).into());
+            .insert("if", (if_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("unless", (unless_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/tags/include_tag.rs b/src/tags/include_tag.rs
@@ -58,12 +58,14 @@ mod test {
 
         let mut options = LiquidOptions::default();
         options.include_source = Box::new(compiler::FilesystemInclude::new(include_path));
-        options.tags.insert("include".to_owned(),
-                            (include_tag as compiler::FnParseTag).into());
-        options.blocks.insert("comment".to_owned(),
+        options
+            .tags
+            .insert("include", (include_tag as compiler::FnParseTag).into());
+        options.blocks.insert("comment",
                               (tags::comment_block as compiler::FnParseBlock).into());
-        options.blocks.insert("if".to_owned(),
-                              (tags::if_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("if", (tags::if_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/tags/interrupt_tags.rs b/src/tags/interrupt_tags.rs
@@ -58,14 +58,18 @@ mod test {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        options.tags.insert("break".to_owned(),
-                            (break_tag as compiler::FnParseTag).into());
-        options.tags.insert("continue".to_owned(),
-                            (continue_tag as compiler::FnParseTag).into());
-        options.blocks.insert("for".to_owned(),
-                              (tags::for_block as compiler::FnParseBlock).into());
-        options.blocks.insert("if".to_owned(),
-                              (tags::if_block as compiler::FnParseBlock).into());
+        options
+            .tags
+            .insert("break", (break_tag as compiler::FnParseTag).into());
+        options
+            .tags
+            .insert("continue", (continue_tag as compiler::FnParseTag).into());
+        options
+            .blocks
+            .insert("for", (tags::for_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("if", (tags::if_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/tags/raw_block.rs b/src/tags/raw_block.rs
@@ -41,8 +41,9 @@ mod test {
 
     fn options() -> LiquidOptions {
         let mut options = LiquidOptions::default();
-        options.blocks.insert("raw".to_owned(),
-                              (raw_block as compiler::FnParseBlock).into());
+        options
+            .blocks
+            .insert("raw", (raw_block as compiler::FnParseBlock).into());
         options
     }
 

diff --git a/src/template.rs b/src/template.rs
@@ -8,7 +8,7 @@ use interpreter::Renderable;
 
 pub struct Template {
     pub(crate) template: interpreter::Template,
-    pub(crate) filters: HashMap<String, interpreter::BoxedValueFilter>,
+    pub(crate) filters: HashMap<&'static str, interpreter::BoxedValueFilter>,
 }
 
 impl Template {