Uses a singleton cache to store the compilation stats

[scinos]

This PR introduces a singleton cache for the compilation stats.

In large Webpack builds generating the stats takes a while and each generated html pays that cost three times. Moreover, if the build uses multiple instances of HtmlWebpackPlugin to generate more than one html file, this cost is payed for each template.

In my local case, I have ~2000 modules and 18 html templates, which normally takes around 175 seconds. With this change, the time goes down to 148 seconds, a 15% improvement.

[jantimon]

Interesting- if to json is that expensive we might also check if we need it at all

[scinos]

For reference, this is what toJSON() does (https://github.com/webpack/webpack/blob/master/lib/Stats.js#L71). tldr: a lot.

[mastilver]

Can be replaced by compilation.chunks

[scinos]

Changing this will change the api of chunksSortMode. Before it received two objects with an array property names. Now they will have a string property name. Example:

diff --git a/spec/BasicSpec.js b/spec/BasicSpec.js
index 27ea085..093f85d 100644
--- a/spec/BasicSpec.js
+++ b/spec/BasicSpec.js
@@ -1414,10 +1414,10 @@ describe('HtmlWebpackPlugin', function () {
       plugins: [
         new HtmlWebpackPlugin({
           chunksSortMode: function (a, b) {
-            if (a.names[0] < b.names[0]) {
+            if (a.name < b.name) {
               return 1;
             }
-            if (a.names[0] > b.names[0]) {
+            if (a.name > b.name) {
               return -1;
             }
             return 0;

Is not documented in the docs (https://github.com/jantimon/html-webpack-plugin#writing-your-own-templates), but it could break some uses. Are you ok with this change?

[mastilver]

Yeah probably not :D

I was wrong here. So is everything still works with getStats() then without the need for toJSON()?

[scinos]

Actually a few properties will change:

Before

{ id: 0,
  rendered: true,
  initial: true,
  entry: true,
  extraAsync: false,
  size: 165,
  names: [ 'main' ],
  files: [ 'bundle.js', 'styles.css' ],
  hash: '6db016c6e99e5efcf9e6',
  parents: [],
  origins: [ [Object] ] }

After

Chunk {
  id: 0,
  ids: [ 0 ],
  name: 'main',
  modules: [ [Object], [Object] ],
  chunks: [],
  parents: [],
  blocks: [],
  origins: [ [Object] ],
  files: [ 'bundle.js', 'styles.css' ],
  rendered: true,
  entry: true,
  initial: true,
  hash: '6db016c6e99e5efcf9e636c868282a89',
  renderedHash: '6db016c6e99e5efcf9e6' }

[mastilver]

only needed to get the hash, which we can get by doing compilation.hash

[mastilver]

I think this is equivalent to just passing compilation

I believe, the only difference between compilation.getStats().toJson() and compilation is that compilation have webpack types while compilation.getStats().toJson() gets us native js types

Not 100% sure on this one thought

[scinos]

From the doc, this should be compilation.getStats(), right?

The following variables are available in the template:

• webpack: the webpack stats object.
  (source https://github.com/jantimon/html-webpack-plugin#writing-your-own-templates)

[mastilver]

Yep you are right 👍

[mastilver]

@jantimon I think you are right here, we don't need toJSON() at all, see my comments

[scinos]

I have removed the calls to toJSON() and everything seems to work (i.e. all tests are passing).

The only problem I see is that using compilation.chunks instead of compilation.getStats().toJson().chunks will change the parameters passed to the function chunksSortMode (see above comments for details).

[scinos]

@mastilver Sorry, I missed your reply above (I blame github notifications).

So the problem is the change

   compiler.plugin('emit', function (compilation, callback) {
     var applyPluginsAsyncWaterfall = self.applyPluginsAsyncWaterfall(compilation);
     // Get all chunks
-    var allChunks = compilation.getStats().toJson().chunks;
+    var allChunks = getStats(compilation).chunks;

If we replace it with compilation.chunks it will change the API of the method chunksSortMode, and compilation.getStats().chunks doesn't exist.

So our options are:

• Use a cached compilation.getStats().toJson() (i.e. the original change)
• Use compilation.chunks and break chunksSortMode API. By exposing a Chunk object we also run into the risk of chunksSortMode calling Chunk methods that change the chunk info when Webpack doesn't expect it anymore.
• Do not change it. After all, the plugin is already 3x faster because we removed two calls to compilation.getStats().toJson()

Thoughts?

[jantimon]

I would prefer not to introduce the cache as it is really hard to understand for other contributors and rather get rit of .toJson() in 3.x

[scinos]

@jantimon so is this ready to be merged for 2.x, or do you want me to change something else?

[jantimon]

The template option is a breaking change so we would rather merge it in 3.x

[scinos]

@jantimon I removed the change that affected the chunksSortMode option, is that the breaking change?

What about merging now the current change (removes 2 out of 3 usages of toJSON(), no breaking changes), and merge the other one in 3.x?

Alternatively, what is the ETA for 3.x? Is there anything I can help with?

[mastilver]

lgtm

I think this is ok to land on 2.x, and to have the other one on 3.x

[scinos]

What is the ETA for release a version of the plugin with this change?

[cyqresig]

When will the fixed version be published?

[gregjacobs]

Has this changed been published yet? I'm experiencing ~25 second rebuild times for 6 html files (project is quite large: ~5200 modules and 10 entry modules)

[asolove]

Edit: my original comment was in error. After profiling my own project I believe the problem could be fixed via the proposal in #780

[lock]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.