Massive performance improvement by using sync requires in NodeJS

[eliseumds]

Hi everyone,

I've seen a pretty big performance improvement by using sync requires in NodeJS. I just tweaked the loadOption function:

if (opts.isNode) {
  // import('aaa') => require('aaa')
  argPath.parent.callee.type = 'Identifier'
  argPath.parent.callee.name = 'require'
}
else {
  // import('aaa') => import(/* webpackChunkName: 'aaa-123' */ 'aaa')
  argPath.addComment('leading', ` webpackChunkName: '${chunkName}' `)
  otherValidMagicComments.forEach(validLeadingComment =>
    argPath.addComment('leading', validLeadingComment.value)
  )
}

It seems like generating multiple chunks, and then using LimitChunksWebpackPlugin to merge them all together for SSR is slow. Does my change make sense? Could it break stuff?

[konpikwastaken]

Can you explain the perf improvement you're seeing? Is this with regards to build time?

An import statement and a require statement are more or less equivalent as webpack transforms it to it's own webpack import statement AFAIK.

[eliseumds]

@konpikwastaken yes, build time. We have a pretty large app with thousands of modules that went from 10s to 2s rebuild time in dev, for example.

Whenever Webpack sees an import statement, it'll split from that point on. It doesn't seem like it's the splitting part that is slow, but rather the joining of modules by LimitChunksPlugin

[konpikwastaken]

Yeah, I get it - I just looked through the LimitWebpackChunksPlugin code as well, so I see what you're saying. On building server bundles though, I thought that import(...) statements became require(...) anyway when you have target:'node' set during build - but perhaps I'm confusing this with what occurs after the chunks are all pieced together.

I can try this out on some projects tomorrow and see if it works as expected - we have some projects using wonky setups with SSR, flushing used chunks, and some crazy webpack path aliasing.

/edit: I mean I think require(..) & import(...) (sync import) with target=node becomes __webpack__require__(...)

[ScriptedAlchemy]

Check out the RFR demo or universal ones if you have any issues.

[ScriptedAlchemy]

Slowness tho, that’s been problematic server build kills.

[eliseumds]

@ScriptedAlchemy our setup was already working great, but development build times were not. I indeed followed the RFR demo since the beginning, but it is a very small application that doesn't represent real-world scenarios; it works, it's fast, but it won't be as fast once it expands to thousands of modules and 100+ chunks. I just did a quick test with 500 extra modules and build time was decreased from 37s to 16s when I disabled LimitChunkCountPlugin. It gets worse and worse the more modules are added.

[ScriptedAlchemy]

I hear you! We all suffer from it.

I might just fork the plugin and use more efficient mapping like fast-transducers

It’s sucks but it’s kind of out of our hands unless we take the hit and try to repair the crap. I hate the plugin. Haaaaaate

[ScriptedAlchemy]

what you can do, is just disable ssr builds in development. What I’ve been doing do one build and turn off HMR on the server

[ScriptedAlchemy]

Can someone PR the README with an example webpack configuration.

This would be so helpful and valuable

@eliseumds

[cdoublev]

I believe that the suggested change will break chunk names flushing, and that adding an example in the README isn't a good idea since it's the RUC responsibility to provide it and having multiple examples in different repositories will make it hard to maintain. Or maybe add a link to Universal Demo.