Skip to content
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 chunks result between multiple entry point html page #802

Closed
wants to merge 9 commits into from

Conversation

jinker
Copy link

@jinker jinker commented Oct 27, 2017

In some time the method compilation.getStats().toJson() take long.
At my project, it takes about 1 to 3s, and I have 155 pages, it will take 155 to 460s total, too slow.
Cache the chunks result between entry points, it will reduce a lot of time.

compiler.plugin('emit', function (compilation, callback) {
    //...
    // Get all chunks
    var allChunks = compilation.getStats().toJson().chunks;
    //....
}
/**
 * Cache chunks result between multiple entry point html page
 */
var hashToChunksMap = {};
//...
compiler.plugin('emit', function (compilation, callback) {
    //...
    // Get all chunks
    var allChunks = self.getAllChunks(compilation);
    //....
}
//....
/**
 * Get all chunks from compilation
 * @param compilation
 * @return {Array}
 */
HtmlWebpackPlugin.prototype.getAllChunks = function (compilation) {
  var allChunks = hashToChunksMap[compilation.hash];
  if (!allChunks) {
    allChunks = compilation.getStats().toJson().chunks;
    hashToChunksMap[compilation.hash] = allChunks;
  }
  return allChunks;
};

Copy link
Collaborator

@mastilver mastilver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR :)
looks good so far, just a few changes

package.json Outdated
@@ -1,6 +1,6 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you revert the changes on this file please

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has been reverted

index.js Outdated
* @param compilation
* @return {Array}
*/
HtmlWebpackPlugin.prototype.getAllChunks = function (compilation) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it should returns only the chunks, it should return the whole JSON so we can use it for

webpack: compilation.getStats().toJson(),

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Push new opt commit, these two places are handled uniformly

Copy link
Collaborator

@mastilver mastilver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm thank you

@sdoomz
Copy link

sdoomz commented Nov 9, 2017

@mastilver what is missing to merge this PR?

@githoniel
Copy link

well I really need this PR, My project have 50 entries now..

@gregjacobs
Copy link

Hey guys, what's the status on this? Failing test?

I'm also wondering (and I mentioned this on another issue before): How come we need to use getStats().toJson() at all? How come the regular object returned by getStats() can't be used?

HtmlWebpackPlugin.prototype.getCompilationStats = function (compilation) {
var stats = compilationHashToStatsMap[compilation.hash];
if (!stats) {
stats = compilation.getStats().toJson();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This toJson call can be passed a number of false options for specific keys to save more time, and limit generation to only the chunk information (which is all that's used by this plugin). You can see an example of that here in another perf PR: https://github.com/jantimon/html-webpack-plugin/pull/825/files#diff-168726dbe96b3ce427e7fedce31bb0bcL67

@gerges-zz
Copy link

getStats().toJSON() is also explicitly called again in the template renderer, so that it could be provided to the template if needed. This causes another pretty large performance hit. There's another PR that allows a plugin parameter which prevents that call: #825

@jinker
Copy link
Author

jinker commented Dec 15, 2017

i don't know why the test failed, what can i do for this pr to merge.@mastilver

@yingye
Copy link

yingye commented Dec 19, 2017

My project have 100+ entries, it's very slow. I really need this PR, to improve pref.

@jinker jinker closed this Dec 26, 2017
@p11o
Copy link

p11o commented May 10, 2018

Please merge.

@lock
Copy link

lock bot commented Jun 9, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants