twitter / hogan.js

A compiler for the Mustache templating language
http://twitter.github.io/hogan.js
Apache License 2.0
5.14k stars 431 forks source link

Performance can be quadratically bad with output length. #218

Closed jimstudt closed 8 years ago

jimstudt commented 9 years ago

In the b(s) method of the Template prototype, the output is collected using a this.buf += s; statement for many tiny strings. If your javascript engine allocates a new string and copies the concatenation of this.buf and s into it each time you will have a quadratic algorithm and a very sad day when you try to make a 4MB output file.

In my case, I addressed this by doing this after I had me template…

// Prepare the template                                                                            
var template = Hogan.compile(htmlSource);

// Replace hogan's buffer writing code to prevent quadratic                                        
// explosion when combined with duktape, version 1.1.1's 'string += string' code.                                 
var chunks = [];
template.b = function(s) { chunks.push(s);};
template.fl = function() { var r = chunks.join(''); chunks = []; return r; };

This is unlikely to be faster than the existing string appending code for short output, but it is a huge difference for large output. In my case I moved from so many minutes that I never saw it complete, to about one second.

Maybe the answer it to document the .b(s) and .fl() methods and include a note about why you might want to do this. Even more performance sensitive people might write directly to a file object in their .b(s) method, for instance.

sayrer commented 9 years ago

"If your javascript engine allocates a new string and copies the concatenation of this.buf and s into it each time"

That looks like a bug in your JavaScript engine. Maybe file an issue on duktape? Here is V8's string buffer growth, for reference:

https://github.com/v8/v8-git-mirror/blob/bd21d72d507625e5caf70f31ee11c8c21ead1651/src/string-builder.h#L142

FWIW, Hogan used to have code exactly like your suggestion for the benefit of older IE, but it was removed as we dropped support for older, more naive implementations:

https://github.com/twitter/hogan.js/blob/master/web/builds/2.0.0/hogan-2.0.0.js#L172