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

[Webpack 5] 3x slowdown caused by _discoverActiveExportsFromOtherStarExports #13107

Closed
redallen opened this issue Apr 9, 2021 · 8 comments · Fixed by #13131
Closed

[Webpack 5] 3x slowdown caused by _discoverActiveExportsFromOtherStarExports #13107

redallen opened this issue Apr 9, 2021 · 8 comments · Fixed by #13131

Comments

@redallen
Copy link

redallen commented Apr 9, 2021

Bug report

What is the current behavior?
Upgrading to Webpack 5 takes 5s to livereload my browser between changes:
image
(the light green blocks at the bottom are calls to getStarReexports and the slow hotspot _discoverActiveExportsFromOtherStarExports)
Webpack 4 takes 3s:
image

If the current behavior is a bug, please provide the steps to reproduce.

  1. git clone -b feat/webpack-5 https://github.com/patternfly/patternfly-org && cd patternfly-org && yarn && yarn start
  2. Wait ~1m for initial webpack compilation.
  3. Save change to packages/v4/src/pages/home.js
  4. Webpack 5 performs worse than Webpack 4, even with module: { unsafeCache: true } and eager-imports-webpack-plugin
798 assets
5779 modules
webpack 5.31.0 compiled successfully in 5051 ms

What is the expected behavior?
Webpack 5 to be faster than webpack 4. Simply changing _discoverActiveExportsFromOtherStarExports to return new Map(); speeds Webpack up 3x:

798 assets
5779 modules
webpack 5.31.0 compiled successfully in 1475 ms

In 95% of cases developers aren't touching star reexports.

I believe this is due to many included barrel files containing thousands of export * from './file';. Is there a way to speedup this _discoverActiveExportsFromOtherStarExports hotspot (or avoid it altogether) besides changing library code to export { list, of, named, exports } from './file';?

Other relevant information:
webpack version: webpack@5.31.0
Node.js version: v14.16.0
Operating System: linux
Relevant: #8644

@sokra
Copy link
Member

sokra commented Apr 9, 2021

I guess that can be improved

@sokra
Copy link
Member

sokra commented Apr 10, 2021

I was unable to run you repro. (Probably windows-related issues, sounds like \ is causing problems in the generated code).

$ yarn start:v4
$ yarn develop:v4
$ yarn workspace patternfly-org-4 develop
$ theme-patternfly-org start
write source files to src/generated
SyntaxError: Bad character escape sequence (1:68)
    at Object.pp$4.raise (C:\Repos\_\patternfly-org\node_modules\acorn\dist\acorn.js:2927:15)
    at Object.pp$9.invalidStringToken (C:\Repos\_\patternfly-org\node_modules\acorn\dist\acorn.js:4922:12)
    at Object.pp$9.readHexChar (C:\Repos\_\patternfly-org\node_modules\acorn\dist\acorn.js:5059:28)

@Z-shh

This comment has been minimized.

@Z-shh

This comment has been minimized.

@redallen
Copy link
Author

I didn't know you used Windows @sokra ! I've updated that branch which should work now.

@sokra
Copy link
Member

sokra commented Apr 13, 2021

I'm looking into fixing that slowness.

If you want an easy fix: 99% of that time is spend in a single file:

node_modules\@patternfly\react-icons\dist\esm\icons\index.js

In contains 1721 lines of export *. The file seem to be autogenerated. I guess it should be easy to generate normal reexports instead:

-export * from './accessible-icon-icon';
+export { AccessibleIconIcon } from './accessible-icon-icon';

export * complexity seem to be O(n²)...
Each export * looks at the exports of all previous export *s.

And we haven't implemented caching for exports that depend on other modules...

@redallen
Copy link
Author

Just changing the star exports in the autogenerated code reduced reload times down to 3s. I'm sure your optimizations will help even more. Thanks @sokra !!

@Z-shh
Copy link

Z-shh commented Apr 17, 2021 via email

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

Successfully merging a pull request may close this issue.

4 participants