fixed memory leak caused by compileDebug flag

[alubbe]

Details and code showing the leak: https://github.com/alubbe/memoryleak
In-browser demo: https://alubbe.github.io/memoryleak/

This leak has been bugging me since I updated node from 0.11.13 to 0.11.14. It has affected all my apps and causes them to take up over a gig of ram per process after a few days. Also, the constant GC makes the app slower and slower with each passing hour.

After forcing the processes to restart every day and also seeing the huge performance regression express received in the last Techempower benchmarks, I got fed up enough to go and track down the issue. First, I started to pre-compile the templates during the build step, and eval'ing the files' contents. This completely fixed any issues I had. So I started working on a new express middleware and just as I was about to publish it, I diffed its implementation to jade's own .compile() and found almost no difference.

That's when it hit me - compileDebug is still enabled by default, even in production. Disabling it via app.locals.compileDebug = false brought jade's performance to the level of my own implementation. So I scrapped my middleware and started to profile jade with heapdumps when compileDebug is enabled. Weirdly enough, the resulting 500mb process had a heapdump of 3mb. According to v8, everything had been successfully GC'ed. So I embarked on an epic 'comment-out-all-the-things!!!' journey and somehow figured out that the issue is with the object allocation into the jade_debug array, specifically, these kinds of lines:

var jade_debug = [{ lineno: 1, filename: "/home/andreas/code/sampleexpress/views/fortunes/index.jade" }];
jade_debug.unshift({ lineno: 0, filename: "/home/andreas/code/sampleexpress/views/layout.jade" });

It seems that since node 0.11.13, v8 has forgotten to identify the shared hidden class of the {lineno: Integer, filename: String} objects. So I made it explicit by conjuring up a DebugItem and voilà, the memory leak is gone.

I'm raising this issue with v8 directly as well (https://code.google.com/p/v8/issues/detail?id=4121), but in the meantime I believe jade should fix it locally because this memory leak exists on node > 0.11.14, 0.12.* and all iojs releases.

I would also like to discuss disabling compileDebug for express' production environment by default, since you already recommend this in the documentation - but that's for a different PR ;)

Looking forward to hear your feedback!

[alubbe]

Quick note, the failing iojs 2.0.2 test was already failing irregardless of my PR and has been fixed here: #1953

[alubbe]

Depending on your stylistic choices, DebugItem could also be implemented as a factory instead of a class, like so:

exports.DebugItem = function DebugItem(lineno, filename) {
  return {
    lineno: lineno,
    filename: filename
  };
}

[ForbesLindesay]

Since this issue manifests itself even on stable node and iojs releases, I think you're right that we should fix this locally.

[ForbesLindesay]

Would you like to be added as a collaborator on jade, to help review and merge pull requests?

[alubbe]

Sure, I'd love to help out!

[ForbesLindesay]

Cool, you're added (You will need to accept the invitation). Please follow these rules:

1. never commit directly to master unless you're only editing documentation, or you're in the process of cutting a new release.
2. never merge your own pull request unless it's had no comments from any collaborators for at least 24 hours.
3. If you cut a new release, make sure you update History.md and follow semver when updating the version number.

If you let me know your npm username I'll add you as an owner so you can release new versions :)

[alubbe]

https://www.npmjs.com/~alubbe
Looking forward to this :)

[ForbesLindesay]

Thanks. I've added you as an owner :)