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

Build time explodes with version higher than 2.8.2 and @material-ui/icons #3640

Closed
valentinbourqui opened this issue Jun 16, 2020 · 3 comments · Fixed by #3641
Closed

Build time explodes with version higher than 2.8.2 and @material-ui/icons #3640

valentinbourqui opened this issue Jun 16, 2020 · 3 comments · Fixed by #3641

Comments

@valentinbourqui
Copy link

valentinbourqui commented Jun 16, 2020

  • Rollup Version: Last/2.9.0 and 2.8.2
  • Operating System (or Browser): Mac
  • Node Version (if applicable): v10.16.3
  • Link to reproduction (IMPORTANT, read below):

https://repl.it/repls/TrustyIntentionalFeeds

Small example projet
https://www.dropbox.com/s/0dhzzz4eq3tc25k/testMe.zip?dl=0

Expected Behavior

Build time must not increase. I don't know if the issue is related to Rollup or @material-ui/icons. I think this is something more generic that happen with @material-ui/icons but could happen with other dependencies too.

Actual Behavior

Time to build increase a lot's since the version 2.9.0 with @material-ui/icons. When I build my application locally on my computer times is multiplied by 10 more or less.I have the same issue with the latest version of Rollup. With the version 2.8.2 and lower it works fine.

I tried to give you an example on REPL.it but on my side it crash because too many files are opened on the same time. Maybe it works for you with an upgraded account. So you can find a very simple project with this issue too.

@lukastaegert
Copy link
Member

lukastaegert commented Jun 16, 2020

Thanks for spotting this one. I must admit I did not experience a 10x performance regression on your example, more like a 25% regression, but it was consistent and noticeable nonetheless. I could actually track it down to some rather unimportant piece of added logic that scaled very badly if a file had many importers (in your example, two files had more than 5000 importers). Potential fix at #3641.

Please give it a spin (see #3641 (comment)) and tell me if this (mostly) fixes the issue for you, it did so for me.

@valentinbourqui
Copy link
Author

Well I think it depends on the machine you have (processor, os, node version, yarn/npm, how your processor is busy with other tasks, etc) but on my side with 2.8.2 it takes 8 seconds and with 2.9.0 50 seconds with the small example project. So you right it's not 10x performance regression sorry but with bigger projects and with the option -w there is a huge impact.

But anyway with your fix #3641 it works again like before. :) Thank you

@lukastaegert
Copy link
Member

Thanks a lot for testing, as you were experiencing the issues more severely, it was important for me to get some feedback from you.

I also want to note that it was extremely helpful that you were able to provide an exact version where the issue occurred and a sufficiently simple example to test against. This allowed me to quickly pinpoint the exact PR that was causing it, then boil it down to changes in a single file and then some trial and error. I'm not sure I would have found it so quickly or at all without this, top notch bug report!

Will release it later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants