Optimize templates. fixes #2033.

Use a single StringBuilder for building template output (as opposed to many string builders appending each other up the tree).
Don't create collections for single items in template source.
Change Appendable's definition to support single StringBuilder.

framework/src/play/src/main/scala/play/api/templates/Templates.scala

@@ -9,27 +9,49 @@ import play.api.http.MimeTypes
 import org.apache.commons.lang3.StringEscapeUtils
 
 /**
- * Appendable content using a StringBuilder.
- * @param buffer StringBuilder to use
+ * Appendable content using a StringBuilder. Either specify elements or text, not both.
+ *
+ * Using an Either[TraversableOnce[A], String] impacts performance in an already
+ * contentious part of code, so it has been done with both parameters instead.
+ *
+ * @param elements Sub elements to traverse when creating the resultant string
+ * @param text Formatted content
  * @tparam A self-type
  */
-abstract class BufferedContent[A <: BufferedContent[A]](private val buffer: StringBuilder) extends Appendable[A] with Content with play.mvc.Content { this: A =>
+abstract class BufferedContent[A <: BufferedContent[A]](protected val elements: TraversableOnce[A], protected val text: String) extends Appendable[A] with Content with play.mvc.Content { this: A =>
+  protected def buildString(builder: StringBuilder) {
+    if (!elements.isEmpty) {
+      elements.foreach { e =>
+        e.buildString(builder)
+      }
+    } else {
+      builder.append(text)
+    }
+  }
 
-  def +=(other: A) = {
-    buffer.append(other.buffer)
-    this
+  /**
+   * This should only ever be called at the top level element
+   * to avoid unneeded memory allocation.
+   */
+  private lazy val builtBody = {
+    val builder = new StringBuilder()
+    buildString(builder)
+    builder.toString
   }
 
-  override def toString = buffer.toString()
+  override def toString = builtBody
 
-  def body = toString
+  def body = builtBody
 
 }
 
 /**
  * Content type used in default HTML templates.
  */
-class Html(buffer: StringBuilder) extends BufferedContent[Html](buffer) {
+class Html private (elements: TraversableOnce[Html], text: String) extends BufferedContent[Html](elements, text) {
+  def this(text: String) = this(Nil, text)
+  def this(elements: TraversableOnce[Html]) = this(elements, "")
+
   /**
    * Content type of HTML.
    */
@@ -45,13 +67,8 @@ object Html {
    * Creates an HTML fragment with initial content specified.
    */
   def apply(text: String): Html = {
-    new Html(new StringBuilder(text))
+    new Html(text)
   }
-
-  /**
-   * Creates an empty HTML fragment.
-   */
-  def empty: Html = new Html(new StringBuilder)
 }
 
 /**
@@ -79,15 +96,28 @@ object HtmlFormat extends Format[Html] {
       case '&' => sb.append("&amp;")
       case c => sb += c
     }
-    new Html(sb)
+    new Html(sb.toString)
   }
 
+  /**
+   * Generate an empty HTML fragment
+   */
+  val empty: Html = new Html("")
+
+  /**
+   * Create an HTML Fragment that holds other fragments.
+   */
+  def fill(elements: TraversableOnce[Html]): Html = new Html(elements)
+
 }
 
 /**
  * Content type used in default text templates.
  */
-class Txt(buffer: StringBuilder) extends BufferedContent[Txt](buffer) {
+class Txt private (elements: TraversableOnce[Txt], text: String) extends BufferedContent[Txt](elements, text) {
+  def this(text: String) = this(Nil, text)
+  def this(elements: TraversableOnce[Txt]) = this(elements, "")
+
   /**
    * Content type of text (`text/plain`).
    */
@@ -103,14 +133,9 @@ object Txt {
    * Creates a text fragment with initial content specified.
    */
   def apply(text: String): Txt = {
-    new Txt(new StringBuilder(text))
+    new Txt(text)
   }
 
-  /**
-   * Creates an empty text fragment.
-   */
-  def empty = new Txt(new StringBuilder)
-
 }
 
 /**
@@ -128,12 +153,25 @@ object TxtFormat extends Format[Txt] {
    */
   def escape(text: String) = Txt(text)
 
+  /**
+   * Generate an empty Txt fragment
+   */
+  val empty: Txt = new Txt("")
+
+  /**
+   * Create an Txt Fragment that holds other fragments.
+   */
+  def fill(elements: TraversableOnce[Txt]): Txt = new Txt(elements)
+
 }
 
 /**
  * Content type used in default XML templates.
  */
-class Xml(buffer: StringBuilder) extends BufferedContent[Xml](buffer) {
+class Xml private (elements: TraversableOnce[Xml], text: String) extends BufferedContent[Xml](elements, text) {
+  def this(text: String) = this(Nil, text)
+  def this(elements: TraversableOnce[Xml]) = this(elements, "")
+
   /**
    * Content type of XML (`application/xml`).
    */
@@ -149,14 +187,9 @@ object Xml {
    * Creates an XML fragment with initial content specified.
    */
   def apply(text: String): Xml = {
-    new Xml(new StringBuilder(text))
+    new Xml(text)
   }
 
-  /**
-   * Create an empty XML fragment.
-   */
-  def empty = new Xml(new StringBuilder)
-
 }
 
 /**
@@ -174,12 +207,25 @@ object XmlFormat extends Format[Xml] {
    */
   def escape(text: String) = Xml(org.apache.commons.lang3.StringEscapeUtils.escapeXml(text))
 
+  /**
+   * Generate an empty XML fragment
+   */
+  val empty: Xml = new Xml("")
+
+  /**
+   * Create an XML Fragment that holds other fragments.
+   */
+  def fill(elements: TraversableOnce[Xml]): Xml = new Xml(elements)
+
 }
 
 /**
  * Type used in default JavaScript templates.
  */
-class JavaScript(buffer: StringBuilder) extends BufferedContent[JavaScript](buffer) {
+class JavaScript private (elements: TraversableOnce[JavaScript], text: String) extends BufferedContent[JavaScript](elements, text) {
+  def this(text: String) = this(Nil, text)
+  def this(elements: TraversableOnce[JavaScript]) = this(elements, "")
+
   /**
    * Content type of JavaScript
    */
@@ -193,7 +239,9 @@ object JavaScript {
   /**
    * Creates a JavaScript fragment with initial content specified
    */
-  def apply(content: String) = new JavaScript(new StringBuilder(content))
+  def apply(text: String) = {
+    new JavaScript(text)
+  }
 }
 
 /**
@@ -212,6 +260,16 @@ object JavaScriptFormat extends Format[JavaScript] {
    */
   def escape(text: String): JavaScript = JavaScript(StringEscapeUtils.escapeEcmaScript(text))
 
+  /**
+   * Generate an empty JavaScript fragment
+   */
+  val empty: JavaScript = new JavaScript("")
+
+  /**
+   * Create an JavaScript Fragment that holds other fragments.
+   */
+  def fill(elements: TraversableOnce[JavaScript]): JavaScript = new JavaScript(elements)
+
 }
 
 /** Defines a magic helper for Play templates. */

framework/src/templates-compiler/src/main/scala/play/templates/ScalaTemplateCompiler.scala

@@ -490,18 +490,26 @@ package play.templates {
 
     }
 
+    protected def displayVisitedChildren(children: Seq[Any]): Seq[Any] = {
+      children.size match {
+        case 0 => Nil
+        case 1 => Nil :+ "_display_(" :+ children :+ ")"
+        case _ => Nil :+ "_display_(Seq[Any](" :+ children :+ "))"
+      }
+    }
+
     def visit(elem: Seq[TemplateTree], previous: Seq[Any]): Seq[Any] = {
       elem match {
         case head :: tail =>
           val tripleQuote = "\"\"\""
           visit(tail, head match {
             case p @ Plain(text) => (if (previous.isEmpty) Nil else previous :+ ",") :+ "format.raw" :+ Source("(", p.pos) :+ tripleQuote :+ text :+ tripleQuote :+ ")"
             case Comment(msg) => previous
-            case Display(exp) => (if (previous.isEmpty) Nil else previous :+ ",") :+ "_display_(Seq[Any](" :+ visit(Seq(exp), Nil) :+ "))"
+            case Display(exp) => (if (previous.isEmpty) Nil else previous :+ ",") :+ displayVisitedChildren(visit(Seq(exp), Nil))
             case ScalaExp(parts) => previous :+ parts.map {
               case s @ Simple(code) => Source(code, s.pos)
               case b @ Block(whitespace, args, content) if (content.forall(_.isInstanceOf[ScalaExp])) => Nil :+ Source(whitespace + "{" + args.getOrElse(""), b.pos) :+ visit(content, Nil) :+ "}"
-              case b @ Block(whitespace, args, content) => Nil :+ Source(whitespace + "{" + args.getOrElse(""), b.pos) :+ "_display_(Seq[Any](" :+ visit(content, Nil) :+ "))}"
+              case b @ Block(whitespace, args, content) => Nil :+ Source(whitespace + "{" + args.getOrElse(""), b.pos) :+ displayVisitedChildren(visit(content, Nil)) :+ "}"
             }
           })
         case Nil => previous

framework/src/templates/src/main/scala/play/api/templates/ScalaTemplate.scala

@@ -35,13 +35,11 @@ package play.templates {
   import reflect.ClassTag
 
   /**
-   * A type that has a binary `+=` operation.
+   * A type that works with BaseScalaTemplate
+   * This used to support +=, but no longer is required to.
+   * @todo Change name to reflect not appendable
    */
-  trait Appendable[T] {
-    def +=(other: T): T
-    override def equals(x: Any): Boolean = super.equals(x) // FIXME Why do we need these overrides?
-    override def hashCode() = super.hashCode()
-  }
+  trait Appendable[T]
 
   /**
    * A template format defines how to properly integrate content for a type `T` (e.g. to prevent cross-site scripting attacks)
@@ -61,22 +59,40 @@ package play.templates {
      * @param text Text to integrate
      */
     def escape(text: String): T
+
+    /**
+     * Generate an empty appendable
+     */
+    def empty: T
+
+    /**
+     * Fill an appendable with the elements
+     */
+    def fill(elements: TraversableOnce[T]): T
   }
 
   case class BaseScalaTemplate[T <: Appendable[T], F <: Format[T]](format: F) {
 
+    // The overloaded methods are here for speed. The compiled templates
+    // can take advantage of them for a 12% performance boost
+    def _display_(x: AnyVal): T = format.escape(x.toString)
+    def _display_(x: String): T = format.escape(x)
+    def _display_(x: Unit): T = format.empty
+    def _display_(x: scala.xml.NodeSeq): T = format.raw(x.toString)
+    def _display_(x: T): T = x
+
     def _display_(o: Any)(implicit ct: ClassTag[T]): T = {
       o match {
         case escaped if escaped != null && escaped.getClass == ct.runtimeClass => escaped.asInstanceOf[T]
-        case () => format.raw("")
-        case None => format.raw("")
+        case () => format.empty
+        case None => format.empty
         case Some(v) => _display_(v)
         case xml: scala.xml.NodeSeq => format.raw(xml.toString)
-        case escapeds: TraversableOnce[_] => escapeds.foldLeft(format.raw(""))(_ += _display_(_))
-        case escapeds: Array[_] => escapeds.foldLeft(format.raw(""))(_ += _display_(_))
+        case escapeds: TraversableOnce[_] => format.fill(escapeds.map(_display_(_)))
+        case escapeds: Array[_] => format.fill(escapeds.toIterator.map(_display_(_)))
         case string: String => format.escape(string)
-        case v if v != null => _display_(v.toString)
-        case _ => format.raw("")
+        case v if v != null => format.escape(v.toString)
+        case _ => format.empty
       }
     }