-
-
Notifications
You must be signed in to change notification settings - Fork 496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cache intermediate NJK template compiles #1529
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I understood, you're basically applying a memoization by looking at the passed extensions and template blocks.
There are some parts, where I think, adding a bit more documentation and picking better variable names could go a long way.
src/Engines/Nunjucks.js
Outdated
let ae = NunjucksLib.Environment.prototype.addExtension; | ||
NunjucksLib.Environment.prototype.addExtension = function (...args) { | ||
let e = args[1]; | ||
e.__id = inc(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this collision proof? Can it happen that the same increment gets assigned to different __id
s? (Perhaps worth to generate a proper UUID here?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is collision proof AFAICT. The 11ty heap lives longer than the invocation of any (and every) sub-template engine, and all extensions are funneled through the Environment's addExtension
method at the end.
My test with this PR: Eleventy v0.11.1
Eleventy HEAD
With this PR #1529
|
Thank you for testing! Were the results correct? No mangled content?
These are very, very slow build times. Is the repo open source by any chance? Can you say more about your build process and hardware/OS? |
Yes, everything is fine. 👍
Indeed, it is 10x what I had not so long ago, because of a very complicated layout I made for my archives, as I explained here: https://nicolas-hoizey.com/articles/2020/10/26/enhancing-archives-navigation-step-1/ The layout has many loops and tests to deal with yearly and monthly archives of 4 different collections: I know that I should be able to enhance it with better collections preparation:
Yes: https://github.com/nhoizey/nicolas-hoizey.com/ If you try to run a build, you will need a few private environment variables, I don't know if I can easily make it work without them. 🤔
As you will see in my package.json, I build CSS from Sass with CLI Dart Sass and PostCSS, JavaScript (both ES6 and IIFE) with CLI Rollup, so Eleventy doesn't have to build these assets. The build time I gave were just for Eleventy. I'm running this on a MacBook Pro 13" 2018 Core i5 with 16 GB RAM, on macOS Big Sur 11.0.1. Big Sur didn't change my built times. HTH |
I’ve tested this against v8.dev @ d3917ab758052a2b7168e5427cf6404f9501fd90. Here’s the results. With eleventy v0.11.1:
With latest eleventy HEAD (9042341):
With this patch:
I’ve confirmed there’s no difference in output — the generated files are bit-by-bit identical in all cases. |
With HTML minification disabled (thanks @slightlyoff !), my tests now give these build times (3 builds for each): With Eleventy v0.11.1:
With Eleventy HEAD (much slower indeed):
With this PR #1529:
|
Really interesting.
Likely due to our use case, head is still faster than 0.11.1. (We have a fairly exotic setup of 11ty where we pass in a nunjucks environment that 11ty then extends.) Behaviour is still as expected. 👍 |
Thanks everyone who has been testing this out! I've rev'd the patch with some additional potential speedups. I might not need to go spelunking to wire up cache invalidation under For comparison, vs. original with default logging (which is slow and adds a lot of variance):
Here's current
Here's
And the same for the new branch:
|
Here are my new results: With Eleventy v0.11.1
With Eleventy HEAD
With this PR
And because I know my responsive images transform is really slow (I have a lot of pages with a lot of images), I also ran Eleventy without it: With Eleventy v0.11.1
With Eleventy HEAD
With this PR
|
Still working well. This time with a smaller project that also inherits a pre-built Nunjucks environment. With Eleventy v0.11.1
With Eleventy HEAD
With this PR
|
One final (I hope!) functional update now re-enables NJK's internal caches which speed things up a little bit more for me. Previous timing was:
And the new result is:
...or another 15% faster. I expect the potential speedup from this change is highly I/O-performance dependent, as its primary effect is to keep NJK from going to disk as often. Versus |
This is excellent—thank you! Will ship with 1.0 |
TL;DR: this patch speeds up my local 11ty-work-only build steps by ~50%
I noticed that running 11ty from upstream is much slower than the 0.11.x branch, which surprised me given some of the work I put in earlier this year. I don't have a root cause analysis, but it got annoying so I decided to see if I couldn't speed things up in my local workflow. This patch picks one of the low-hanging fruit I mused at back in May, caching intermediate template compiles after some profiling showed that repeated application of the same templates across many data files was causing NJK compilation to show up as heavy in my local blog project.
The benefits here are NJK specific (sorry, other template engines!) and probably brittle. Feedback on the patch appreciated.
Before (
11ty/eleventy
HEAD)After (NJK template caching branch)