Consider faster alternatives to lodash/isPlainObject

[jeffposnick]

Is your feature request related to a problem? Please describe.
I'm taking a look at the build performance of https://github.com/GoogleChrome/web.dev, which is a fairly large (~3300 files) 11ty site.

Describe the solution you'd like
A detailed description of one of my initial findings is at GoogleChrome/web.dev#7339; the takeaway is that

eleventy/src/Util/Merge.js

Line 1 in 08fe54e

const isPlainObject = require("lodash/isPlainObject");

ends up being a very big performance hotspot for our build. Using module-alias to swap out lodash/isPlainObject for the implementation from typechecker, without making any other changes, takes our build time down from 55 seconds to 40 seconds.

It would be great if someone from the core 11ty team could try making this change as well and see if they experience similar performance improvements on large sites, without any unintended changes. I'm assuming the 11ty test suite would pick up any compatibility issues.

Describe alternatives you've considered
I can't guarantee whether the implementation in typechecker is 100% compatible with lodash/isPlainObject, or whether typechecker's implementation is faster than a few other implementations that I found on npm.

Additional context
As mentioned, there are details in GoogleChrome/web.dev#7339 of the methodology for this investigation and steps to reproduce what I tried.

[jeffposnick]

It's also worth noting that even with the performance boost we get from swapping out isPlainObject, https://github.com/11ty/eleventy/blob/master/src/Util/Merge.js accounts for a large chunk of our build's execution time.

We are pre-v1.0.0 but explicitly opted-in to data deep merge. I believe that it's on by default for everyone post-v1.0.0, so maybe it's worth taking a pass at optimizing the merge algorithm? I could imagine other sites that didn't have data deep merge turned on seeing a slowdown in v1.0.0 if they suddenly start hitting this code path.

[jeffposnick]

One more data point—I did some basic logging and found that for our ~3300 page site,

eleventy/src/Util/Merge.js

Line 4 in 08fe54e

function getMergedItem(target, source, parentKey) {

ends up being called (recursively) ~88 million times during our build process. That's likely why even a small improvement in the performance characteristics of the function, like a slightly more efficient isPlainObject(), ends up having a significant impact on our builds.

[nhoizey]

88 million times! 🤯

[zachleat]

Whew, alright! A few things here!

(and a celebration of this as the first issue post-Netlify sponsorship 🥳)

I opened #2219 in service of this! Very happy to entertain reviews over there!

I looked at a few different implementations here and took inspiration from is-what, typechecker, and lodash. I decided that it wasn’t quite worth using an external dependency for this and pulled together code (and tests!) from a few different implementations.

Test suites are passing fine on our end.

I would note that, in the larger picture, this likely isn’t strictly limited to deep data merge, it’s used heavily in permalink and likely more expensively in Computed Data—are y’all making extensive use of these features?

Found in:

src/ComputedDataProxy.js:
src/Template.js:
src/TemplateBehavior.js:
src/TemplateMap.js:
src/TemplatePermalink.js:
src/Plugins/RenderPlugin.js:
src/Util/Merge.js:

Couple of things I would love to see here:

• A test on web.dev using that branch to test the performance. If it doesn’t get the results we want, maybe we can at least talk about getting access to that repo so that I can test with the real code 😅
• We might also want to post a few follow up issues here, too? My hunch is that a performance review of Computed Data is likely warranted, and maybe improvements to Merge.js too

[jeffposnick]

Performance comparison

We've been slow to move to v1.0.0, so doing a direct comparison requires hacking at our dependencies a little bit.

If I manually switch to update to the tagged v1.0.0 release, and run npm run eleventy from our repo, using node v14, I see

$ npm run eleventy

> web.dev@3.0.0 eleventy /Users/.../git/web.dev
> eleventy --quiet

Eleventy is building, please wait…
[11ty] Wrote 3378 files in 45.67 seconds (13.5ms each, v1.0.0)

If I switch to your branch locally and run things again, I see:

$ npm run eleventy

> web.dev@3.0.0 eleventy /Users/.../git/web.dev
> eleventy --quiet

Eleventy is building, please wait…
[11ty] Wrote 3378 files in 32.70 seconds (9.7ms each, v1.0.1-canary.2)

So, definitely better! 💥

Trying this yourself

If you want to try it yourself, you shouldn't have to do much more than clone https://github.com/GoogleChrome/web.dev, hack the package.json a bit to pull in a local copy of 11ty (e.g. "@11ty/eleventy": "file://../eleventy", or use npm link), and then execute npm run eleventy.

We're currently using node v14 throughout the project, and the overall build times (irrespective of this patch) get better when using node v16. So that's something on our end we're looking to upgrade.

Other codepaths

I specifically saw Merge in the callstack when profiling what had the top execution time for our build, so I think it's swapping out that specific usage that will really benefit us. It's good to hear that there are other code paths that might improve as well!

I actually am looking after this codebase on a short-term basis, and am not 100% sure what our Computed Data and permalink usage is like.

Further Merge improvements

I had done a bit of code golf to see if some calls to isPlainObject could be avoided, e.g. when we know the object is an array, or know it's {}. You can take a look, but with a more optimized isPlainObject in place, the execution time is pretty much the same before or after those optimizations. So... 🤷

[zachleat]

Excellent, thank you @jeffposnick!

I think we can maybe consider this one resolved for now? I might file another issue to do more performance investigation on web.dev.

Looking at a bigger picture, I’d love to get a test corpus of large open source projects to performance test releases on, but I don’t know if that makes sense to do with how tied some of these codebases can be to individual versions.

I do have https://github.com/11ty/eleventy-benchmark in place for some very rudimentary “lots of files” testing, but the test projects there are very uniform.

If I might recommend further investigation, I’d suggest looking for eleventyComputed in the data cascade and looking if the usage is template strings versus JS functions: note the warning on https://www.11ty.dev/docs/data-computed/#using-a-template-string

[nhoizey]

@zachleat my own website builds a lot of files (a little less than web.dev):

Copied 928 files / Wrote 1820 files in 39.31 seconds (21.6ms each, v0.12.1)

(Whoops, this one is not yet on v1… 😅)

So if you have instructions on how and where to do performance tests, I can try.

[zachleat]

Filed a new one over at #2220 (cc @nhoizey too)

And internal isPlainObject is slated to ship with 1.0.1—thank you!