perf(render): Use a Write

This is hopefully better than string concatenation, especially if you can write directly to file. Fixes #187 BREAKING CHANGE: `parser.render` now takes an `std::io::Write` rather than return a `String`.
cobalt-org · Sep 5, 2018 · 946bc74 · 946bc74
1 parent d8a455a
commit 946bc74
Show file tree

Hide file tree

Showing 16 changed files with 127 additions and 81 deletions.
diff --git a/src/interpreter/output.rs b/src/interpreter/output.rs
@@ -1,4 +1,5 @@
 use std::fmt;
+use std::io::Write;
 
 use itertools;
 
@@ -53,9 +54,10 @@ impl fmt::Display for Output {
 }
 
 impl Renderable for Output {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()> {
         let entry = self.apply_filters(context)?;
-        Ok(Some(entry.to_string()))
+        write!(writer, "{}", entry).chain("Failed to render")?;
+        Ok(())
     }
 }
 

diff --git a/src/interpreter/renderable.rs b/src/interpreter/renderable.rs
@@ -1,14 +1,11 @@
 use std::fmt::Debug;
+use std::io::Write;
 
 use error::Result;
-
 use super::Context;
 
 /// Any object (tag/block) that can be rendered by liquid must implement this trait.
 pub trait Renderable: Send + Sync + Debug {
     /// Renders the Renderable instance given a Liquid context.
-    /// The Result that is returned signals if there was an error rendering,
-    /// the Option<String> that is wrapped by the Result will be None if
-    /// the render has run successfully but there is no content to render.
-    fn render(&self, context: &mut Context) -> Result<Option<String>>;
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()>;
 }
diff --git a/src/interpreter/template.rs b/src/interpreter/template.rs
@@ -1,5 +1,6 @@
-use error::Result;
+use std::io::Write;
 
+use error::Result;
 use super::Context;
 use super::Renderable;
 
@@ -9,12 +10,9 @@ pub struct Template {
 }
 
 impl Renderable for Template {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
-        let mut buf = String::new();
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()> {
         for el in &self.elements {
-            if let Some(ref x) = el.render(context)? {
-                buf = buf + x;
-            }
+            el.render(writer, context)?;
 
             // Did the last element we processed set an interrupt? If so, we
             // need to abandon the rest of our child elements and just
@@ -24,7 +22,7 @@ impl Renderable for Template {
                 break;
             }
         }
-        Ok(Some(buf))
+        Ok(())
     }
 }
 

diff --git a/src/interpreter/text.rs b/src/interpreter/text.rs
@@ -1,5 +1,6 @@
-use error::Result;
+use std::io::Write;
 
+use error::{Result, ResultLiquidChainExt};
 use super::Context;
 use super::Renderable;
 
@@ -9,8 +10,9 @@ pub struct Text {
 }
 
 impl Renderable for Text {
-    fn render(&self, _context: &mut Context) -> Result<Option<String>> {
-        Ok(Some(self.text.to_owned()))
+    fn render(&self, writer: &mut Write, _context: &mut Context) -> Result<()> {
+        write!(writer, "{}", &self.text).chain("Failed to render")?;
+        Ok(())
     }
 }
 

diff --git a/src/interpreter/variable.rs b/src/interpreter/variable.rs
@@ -1,8 +1,9 @@
 use std::fmt;
+use std::io::Write;
 
 use itertools;
 
-use error::Result;
+use error::{Result, ResultLiquidChainExt};
 use value::Index;
 
 use super::Context;
@@ -38,9 +39,10 @@ impl fmt::Display for Variable {
 }
 
 impl Renderable for Variable {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()> {
         let value = context.get_val_by_index(self.indexes.iter())?;
-        Ok(Some(value.to_string()))
+        write!(writer, "{}", value).chain("Failed to render")?;
+        Ok(())
     }
 }
 

diff --git a/src/tags/assign_tag.rs b/src/tags/assign_tag.rs
@@ -1,5 +1,6 @@
-use error::Result;
+use std::io::Write;
 
+use error::Result;
 use compiler::LiquidOptions;
 use compiler::ResultLiquidExt;
 use compiler::Token;
@@ -21,12 +22,12 @@ impl Assign {
 }
 
 impl Renderable for Assign {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
+    fn render(&self, _writer: &mut Write, context: &mut Context) -> Result<()> {
         let value = self.src
             .apply_filters(context)
             .trace_with(|| self.trace().into())?;
         context.set_global_val(self.dst.to_owned(), value);
-        Ok(None)
+        Ok(())
     }
 }
 

diff --git a/src/tags/capture_block.rs b/src/tags/capture_block.rs
@@ -1,5 +1,9 @@
-use error::{Result, ResultLiquidExt};
+use std::io::Write;
+use std::io;
+use std::result;
+use std::str;
 
+use error::{Result, ResultLiquidExt};
 use compiler::Element;
 use compiler::LiquidOptions;
 use compiler::Token;
@@ -9,6 +13,35 @@ use interpreter::Renderable;
 use interpreter::Template;
 use value::Value;
 
+struct CapturingWriter<'a> {
+    captured: Vec<u8>,
+    writer: &'a mut Write,
+}
+
+impl<'a> CapturingWriter<'a> {
+    fn new(writer: &'a mut Write) -> Self {
+        Self {
+            captured: Default::default(),
+            writer,
+        }
+    }
+
+    fn captured(&self) -> &[u8] {
+        &self.captured
+    }
+}
+
+impl<'a> Write for CapturingWriter<'a> {
+    fn write(&mut self, buf: &[u8]) -> result::Result<usize, io::Error> {
+        self.captured.extend(buf);
+        self.writer.write(buf)
+    }
+
+    fn flush(&mut self) -> result::Result<(), io::Error> {
+        self.writer.flush()
+    }
+}
+
 #[derive(Debug)]
 struct Capture {
     id: String,
@@ -22,14 +55,15 @@ impl Capture {
 }
 
 impl Renderable for Capture {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
-        let output = self.template
-            .render(context)
-            .trace_with(|| self.trace().into())?
-            .unwrap_or_else(|| "".to_owned());
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()> {
+        let mut writer = CapturingWriter::new(writer);
+        self.template
+            .render(&mut writer, context)
+            .trace_with(|| self.trace().into())?;
 
+        let output = str::from_utf8(writer.captured()).expect("render only writes UTF-8").to_owned();
         context.set_global_val(self.id.to_owned(), Value::scalar(output));
-        Ok(None)
+        Ok(())
     }
 }
 

diff --git a/src/tags/case_block.rs b/src/tags/case_block.rs
@@ -1,7 +1,8 @@
+use std::io::Write;
+
 use itertools;
 
 use error::{Error, Result, ResultLiquidExt};
-
 use compiler::Element;
 use compiler::LiquidOptions;
 use compiler::Token;
@@ -52,26 +53,26 @@ impl Case {
 }
 
 impl Renderable for Case {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()> {
         let value = self.target.evaluate(context)?;
         for case in &self.cases {
             if case.evaluate(&value, context)? {
                 return case.template
-                    .render(context)
+                    .render(writer, context)
                     .trace_with(|| case.trace().into())
                     .trace_with(|| self.trace().into())
                     .context_with(|| (self.target.to_string().into(), value.to_string()));
             }
         }
 
         if let Some(ref t) = self.else_block {
-            return t.render(context)
+            return t.render(writer, context)
                 .trace_with(|| "{{% else %}}".to_owned().into())
                 .trace_with(|| self.trace().into())
                 .context_with(|| (self.target.to_string().into(), value.to_string()));
         }
 
-        Ok(None)
+        Ok(())
     }
 }
 

diff --git a/src/tags/comment_block.rs b/src/tags/comment_block.rs
@@ -1,5 +1,6 @@
-use error::Result;
+use std::io::Write;
 
+use error::Result;
 use compiler::Element;
 use compiler::LiquidOptions;
 use compiler::Token;
@@ -10,8 +11,8 @@ use interpreter::Renderable;
 struct Comment;
 
 impl Renderable for Comment {
-    fn render(&self, _context: &mut Context) -> Result<Option<String>> {
-        Ok(None)
+    fn render(&self, _writer: &mut Write, _context: &mut Context) -> Result<()> {
+        Ok(())
     }
 }
 

diff --git a/src/tags/cycle_tag.rs b/src/tags/cycle_tag.rs
@@ -1,7 +1,8 @@
-use itertools;
+use std::io::Write;
 
-use error::{Result, ResultLiquidExt};
+use itertools;
 
+use error::{Result, ResultLiquidChainExt, ResultLiquidExt};
 use compiler::LiquidOptions;
 use compiler::Token;
 use compiler::{consume_value_token, unexpected_token_error, value_token};
@@ -25,11 +26,14 @@ impl Cycle {
 }
 
 impl Renderable for Cycle {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()> {
         let value = context
             .cycle_element(&self.name, &self.values)
             .trace_with(|| self.trace().into())?;
-        Ok(value.map(|v| v.to_string()))
+        if let Some(ref value) = value {
+            write!(writer, "{}", value).chain("Failed to render")?;
+        }
+        Ok(())
     }
 }
 

diff --git a/src/tags/for_block.rs b/src/tags/for_block.rs
@@ -1,4 +1,5 @@
 use std::fmt;
+use std::io::Write;
 use std::slice::Iter;
 
 use itertools;
@@ -110,7 +111,7 @@ fn for_slice(range: &mut [Value], limit: Option<usize>, offset: usize, reversed:
 }
 
 impl Renderable for For {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()> {
         let mut range = self.range
             .evaluate(context)
             .trace_with(|| self.trace().into())?;
@@ -119,16 +120,13 @@ impl Renderable for For {
         match range.len() {
             0 => {
                 if let Some(ref t) = self.else_template {
-                    t.render(context)
+                    t.render(writer, context)
                         .trace_with(|| "{{% else %}}".to_owned().into())
-                        .trace_with(|| self.trace().into())
-                } else {
-                    Ok(None)
+                        .trace_with(|| self.trace().into())?;
                 }
             }
 
             range_len => {
-                let mut ret = String::default();
                 context.run_in_scope(|mut scope| {
                     let mut helper_vars = Object::new();
                     helper_vars.insert("length".into(), Value::scalar(range_len as i32));
@@ -147,13 +145,11 @@ impl Renderable for For {
 
                         scope.set_val("forloop", Value::Object(helper_vars.clone()));
                         scope.set_val(self.var_name.to_owned(), v.clone());
-                        let inner = self.item_template
-                            .render(&mut scope)
+                        self.item_template
+                            .render(writer, &mut scope)
                             .trace_with(|| self.trace().into())
                             .context_with(|| (self.var_name.clone().into(), v.to_string()))
-                            .context("index", &(i + 1))?
-                            .unwrap_or_else(String::new);
-                        ret = ret + &inner;
+                            .context("index", &(i + 1))?;
 
                         // given that we're at the end of the loop body
                         // already, dealing with a `continue` signal is just
@@ -163,11 +159,11 @@ impl Renderable for For {
                             break;
                         }
                     }
-
-                    Ok(Some(ret))
-                })
+                    Ok(())
+                })?;
             }
         }
+        Ok(())
     }
 }
 

diff --git a/src/tags/if_block.rs b/src/tags/if_block.rs
@@ -1,4 +1,5 @@
 use std::fmt;
+use std::io::Write;
 
 use error::{Result, ResultLiquidExt};
 
@@ -131,21 +132,23 @@ impl Conditional {
 }
 
 impl Renderable for Conditional {
-    fn render(&self, context: &mut Context) -> Result<Option<String>> {
-        let condition = self.compare(context).trace_with(|| self.trace().into());
-        if condition? {
+    fn render(&self, writer: &mut Write, context: &mut Context) -> Result<()> {
+        let condition = self.compare(context).trace_with(|| self.trace().into())?;
+        if condition {
             self.if_true
-                .render(context)
-                .trace_with(|| self.trace().into())
+                .render(writer, context)
+                .trace_with(|| self.trace().into())?;
         } else {
             match self.if_false {
                 Some(ref template) => template
-                    .render(context)
+                    .render(writer, context)
                     .trace_with(|| "{{% else %}}".to_owned().into())
-                    .trace_with(|| self.trace().into()),
-                _ => Ok(None),
+                    .trace_with(|| self.trace().into())?,
+                _ => (),
             }
         }
+
+        Ok(())
     }
 }