From 3d93928b2a9ac378dbb3ca8bd097b1ed7112620f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 13 Jan 2018 17:22:18 -0700 Subject: [PATCH] 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 --- src/compiler/options.rs | 4 ++-- src/compiler/parser.rs | 19 ++++++++++--------- src/interpreter/context.rs | 8 ++++---- src/parser.rs | 26 ++++++++++++++++---------- src/tags/assign_tag.rs | 15 +++++++++------ src/tags/capture_block.rs | 5 +++-- src/tags/case_block.rs | 5 +++-- src/tags/comment_block.rs | 5 +++-- src/tags/cycle_tag.rs | 5 +++-- src/tags/for_block.rs | 5 +++-- src/tags/if_block.rs | 7 ++++--- src/tags/include_tag.rs | 12 +++++++----- src/tags/interrupt_tags.rs | 20 ++++++++++++-------- src/tags/raw_block.rs | 5 +++-- src/template.rs | 2 +- 15 files changed, 83 insertions(+), 60 deletions(-) diff --git a/src/compiler/options.rs b/src/compiler/options.rs index 945f3e8b4..d62984e11 100644 --- 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, - pub tags: HashMap, + pub blocks: HashMap<&'static str, BoxedBlockParser>, + pub tags: HashMap<&'static str, BoxedTagParser>, pub include_source: Box, } diff --git a/src/compiler/parser.rs b/src/compiler/parser.rs index 8618c28b7..d7d533482 100644 --- a/src/compiler/parser.rs +++ b/src/compiler/parser.rs @@ -53,8 +53,8 @@ fn parse_expression(tokens: &[Token], options: &LiquidOptions) -> Result { - 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, 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" 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, }; 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 = ["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 index f09e5329e..bae9c65f4 100644 --- a/src/interpreter/context.rs +++ b/src/interpreter/context.rs @@ -29,7 +29,7 @@ pub struct Context { cycles: HashMap, // Public for backwards compatability - filters: HashMap, + filters: HashMap<&'static str, BoxedValueFilter>, } impl Context { @@ -43,7 +43,7 @@ impl Context { self } - pub fn with_filters(mut self, filters: HashMap) -> 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 index dd4133dff..a6f0be910 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -12,9 +12,9 @@ use super::Template; #[derive(Default, Clone)] pub struct ParserBuilder { - blocks: HashMap, - tags: HashMap, - filters: HashMap, + blocks: HashMap<&'static str, compiler::BoxedBlockParser>, + tags: HashMap<&'static str, compiler::BoxedTagParser>, + filters: HashMap<&'static str, interpreter::BoxedValueFilter>, include_source: Option>, } @@ -129,20 +129,26 @@ impl ParserBuilder { } /// Inserts a new custom block into the parser - pub fn block>(mut self, name: &str, block: B) -> Self { - self.blocks.insert(name.to_owned(), block.into()); + pub fn block>(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>(mut self, name: &str, tag: T) -> Self { - self.tags.insert(name.to_owned(), tag.into()); + pub fn tag>(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>(mut self, name: &str, filter: F) -> Self { - self.filters.insert(name.to_owned(), filter.into()); + pub fn filter>(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, + filters: HashMap<&'static str, interpreter::BoxedValueFilter>, } impl Parser { diff --git a/src/tags/assign_tag.rs b/src/tags/assign_tag.rs index c9184d911..9e8835d0d 100644 --- 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 index 76e46299d..de336144f 100644 --- 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 index 98283539d..d7794be38 100644 --- 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 index f4305adab..2625ef8c4 100644 --- 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 index d0128b288..2e62375ad 100644 --- 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 index bfaa91f3a..5141e7610 100644 --- 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 index 92ccd7d90..d7fd7c19d 100644 --- 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 index da34b2cf4..10a541ab4 100644 --- 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 index c9f8d0f7a..febbf51af 100644 --- 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 index 7901ad153..7ad74c56e 100644 --- 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 index 0a0446600..4f69ac117 100644 --- 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, + pub(crate) filters: HashMap<&'static str, interpreter::BoxedValueFilter>, } impl Template {