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

Improve perf by cacheing chunks? #780

Closed
asolove opened this issue Sep 10, 2017 · 12 comments
Closed

Improve perf by cacheing chunks? #780

asolove opened this issue Sep 10, 2017 · 12 comments

Comments

@asolove
Copy link

asolove commented Sep 10, 2017

Hi there! Thanks for this plugin, it's been super useful for us!

Unfortunately, in one project we experienced much lengthier-than-expected build times as we added more html entries to our config. After profiling, I found that this plugin currently generates the full webpack stats object once per html entry point.

screen shot 2017-09-09 at 7 39 04 pm

In this example during a dev rebuild, we have 13 html entries and there are 13 calls to getStats().toJson(). Each one takes ~100ms, so just that ends up being more than half of the runtime of the build.

For larger projects with many entry points and modules, this creates a big slowdown. (There are a number of other reports of this, though too vague to be sure this is really the issue, over in a previous perf PR: #723)

If I understand this plugin's logic, I think it is only necessary to generate the stats object once, and also that we could pass in options to only generate the chunks part rather than all the other data.

Would you be interested in a PR to cache the chunks and minimize the stats requested? Any ideas on where that cache could be placed?

Thanks again!

@mastilver
Copy link
Collaborator

I don't think having some sort of cache between different instances of the plugin will be easy and bug-free

I guess this can be achieved by doing:

#558

Generate multiple html files (e.g. by using [name] as output name)

@jantimon Can you give us the API you had in mind for that?

@mastilver
Copy link
Collaborator

I wonder if we can just pass in an array (and keep the old behavior of an object) as a config?

@asolove
Copy link
Author

asolove commented Sep 10, 2017

@mastilver thanks for your answers! Can you explain briefly why sharing a cache of chunks would be buggy? In my light testing on our repo, the data fetched from stats about chunks was always the same between instances. But obviously I don’t know this plugin super well so I bet you’re right, and I just don’t understand yet.

@mastilver
Copy link
Collaborator

@asolove how would you deal with multiple compilation (when you export an array from your webpack config) ?

@gregjacobs
Copy link

gregjacobs commented Sep 11, 2017

Question for you guys: why does the plugin need the entire stats object as json? Couldn't it navigate the original object references to find what it needs?

As an aside, we're experiencing the same type of issue, regenerating 6 html files in a 5000+ module project takes ~25sec on our work machines.

@gregjacobs
Copy link

I just noticed that the entire stats json is passed to the html template as webpack. Perhaps we could add an option to the plugin to skip that stats generation if webpack is unused on the template.

@gregjacobs
Copy link

Even better than an option to disable the stats if unused in the html template would be to make the variable webpack a getter, and only generate the stats json if it is accessed.

@mastilver
Copy link
Collaborator

make the variable webpack a getter, and only generate the stats json if it is accessed.

@gregjacobs That's a really good idea! :)

@reubn
Copy link

reubn commented Nov 18, 2017

#802

@niftymonkey
Copy link

Here's another attempt at addressing this issue. Minimizes the work done at toJson time in the emit handler and also provides a mechanism for getting rid of the second call in the template generation if you don't need it: #825

@jantimon
Copy link
Owner

#953 removes getStats().toJson() to improve this - please let me know if there are any problems with the proposed solution.

@jantimon
Copy link
Owner

jantimon commented Jun 6, 2018

#967 improves the performance even more and according to first measurements has a quite good improvement: #953 (comment)

We are still trying to finalize it in #967 - please add comments there or open a new issue if it is not related to #967

@jantimon jantimon closed this as completed Jun 6, 2018
Repository owner locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants