fix: Faster method of building dependencies array

[paddyobrien]

Motivation

Fixes #797

Summary

I'm pretty surprised that this has any impact, I don't have a good rationale for why but it appears to be dramatically faster to update the array in place.

Using the demo repo linked in the above issue without this change:

$ webpack
asset app.js 961 KiB [compared for emit] (name: app) 1 related asset
asset styles.css 8.35 KiB [compared for emit] (name: app) 1 related asset
Entrypoint app 969 KiB (1.11 MiB) = styles.css 8.35 KiB app.js 961 KiB 2 auxiliary assets
orphan modules 390 bytes [orphan] 3 modules
runtime modules 670 bytes 3 modules
modules by path ./node_modules/ 947 KiB
  modules by path ./node_modules/scheduler/ 32.4 KiB 4 modules
  modules by path ./node_modules/react/ 59.4 KiB 2 modules
  modules by path ./node_modules/react-dom/ 840 KiB 2 modules
  modules by path ./node_modules/@linaria/ 4.34 KiB 2 modules
  modules by path ./node_modules/@emotion/ 4.41 KiB 2 modules
  modules by path ./node_modules/prop-types/ 4 KiB 2 modules
  ./node_modules/object-assign/index.js 2.06 KiB [built] [code generated]
modules by path ./.linaria-cache/src/*.css 50 bytes (javascript) 8.31 KiB (css/mini-extract)
  ./.linaria-cache/src/App.linaria.css 50 bytes [built] [code generated]
  css ./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[1].use[1]!./.linaria-cache/src/App.linaria.css 8.31 KiB [built] [code generated]
./src/App.js 4.68 KiB [built] [code generated]
webpack 5.42.0 compiled successfully in 23904 ms

And with this change:

$ webpack
asset app.js 961 KiB [emitted] (name: app) 1 related asset
asset styles.css 8.35 KiB [compared for emit] (name: app) 1 related asset
Entrypoint app 969 KiB (1.11 MiB) = styles.css 8.35 KiB app.js 961 KiB 2 auxiliary assets
orphan modules 390 bytes [orphan] 3 modules
runtime modules 670 bytes 3 modules
modules by path ./node_modules/ 947 KiB
  modules by path ./node_modules/scheduler/ 32.4 KiB 4 modules
  modules by path ./node_modules/react/ 59.4 KiB 2 modules
  modules by path ./node_modules/react-dom/ 840 KiB 2 modules
  modules by path ./node_modules/@linaria/ 4.34 KiB 2 modules
  modules by path ./node_modules/@emotion/ 4.41 KiB 2 modules
  modules by path ./node_modules/prop-types/ 4 KiB 2 modules
  ./node_modules/object-assign/index.js 2.06 KiB [built] [code generated]
modules by path ./.linaria-cache/src/*.css 50 bytes (javascript) 8.31 KiB (css/mini-extract)
  ./.linaria-cache/src/App.linaria.css 50 bytes [built] [code generated]
  css ./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[1].use[1]!./.linaria-cache/src/App.linaria.css 8.31 KiB [built] [code generated]
./src/App.js 4.68 KiB [built] [code generated]
webpack 5.42.0 compiled successfully in 1874 ms

[callstack-bot]

Hey @paddyobrien, thank you for your pull request 🤗.
The coverage report for this branch can be viewed here.

[Anber]

Hi @paddyobrien
This is an incredible catch! I can't explain why concat+reduce has such an incredible complexity when it runs in webpack5 context. Probably, webpack 5 somewhat tries to manage memory allocations or garbage-collections which reduce+concat produces quite a lot.
Anyway, thank you so much!

[paddyobrien]

Thanks for merging @Anber I was really surprised this was so much faster but a process of elimination led me to it and I was left with no other conclusion than to change these lines.

Would it be possible to cut another beta release so we can check with our production app if there are any other performance regressions?

[Anber]

@paddyobrien I'll publish the next beta later today.

[eoin]

@paddyobrien and I weren't very satisfied with our understanding of the root cause here so we spent some time digging 😄
It turned out to be pretty interesting. Here's a writeup.

[rcstr]

I enjoyed a lot the write-up @eoin. thanks a lot!

[davemilter]

@paddyobrien

I'm pretty surprised that this has any impact, I don't have a good rationale for why but it appears to be dramatically faster to update the array in place.

But this code on each iteration create new Array and copy result to it:

return nodes.reduce(
  (acc, node) => acc.concat(Array.from(this.dependencies.get(node) || [])),

with length from one 1 to n, where n length of nodes.
So this is n * (n + 1) operation, in other words complexity of initial algorithm is O(n^2).
So obviously the new algorithm with complexity O(n) is n times faster.

So it should be pretty surprised if there are dramatically impact of such changes.
And gives thanks to v8 who some times can optimize such slow code.