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

Browsers.parse takes 70% of the autoprefixer time #1256

Closed
Knagis opened this issue Oct 15, 2019 · 14 comments
Closed

Browsers.parse takes 70% of the autoprefixer time #1256

Knagis opened this issue Oct 15, 2019 · 14 comments
Assignees
Labels

Comments

@Knagis
Copy link

@Knagis Knagis commented Oct 15, 2019

We use autoprefixer with a single .browserslistrc file in root. However, looking at profile trace for our webpack build, it shows that autoprefixer keeps reloading the file every time postcss plugin triggers. Overall it is not as much - 5 seconds total (out of 1 minute build) but for autoprefixer on its own browser list parsing takes >70% of the time.

image

@ai

This comment has been minimized.

Copy link
Member

@ai ai commented Oct 15, 2019

Are we talking about single Autoprefixer run or on processing multiple files?

Browserslist has an in-memory cache for config lookup https://github.com/browserslist/browserslist/blob/c806b2441179b15cee6eb6a783e754be6b332d20/node.js#L241-L244

@Knagis

This comment has been minimized.

Copy link
Author

@Knagis Knagis commented Oct 15, 2019

Our case is processing many files in a single webpack build.

Looking at the cache code it seems to cache based on CSS file path. We have files in many folders - but all resolve to the same browser list file.

@ai

This comment has been minimized.

Copy link
Member

@ai ai commented Oct 17, 2019

Cache should be based on folder path.

The problem is that you can put .browserslistrc to any folder to override global Browserslist config.

But I have an idea how to speed this search a little.

@ai

This comment has been minimized.

Copy link
Member

@ai ai commented Oct 17, 2019

Hope it improved Browserslist caching browserslist/browserslist@86caf2b

@ai

This comment has been minimized.

Copy link
Member

@ai ai commented Oct 17, 2019

Can you try to update Browserslist to 4.7.1 (double check that you updated all version by npm ls) and check performance again?

@Knagis

This comment has been minimized.

Copy link
Author

@Knagis Knagis commented Oct 22, 2019

With 4.7.1 profiler still shows the same distribution.

The change seems to store the parsed config for each parent folder, however reading the cache only looks at the current one. If CSS files are all in leaf folders, then the change didn't provide any improvement.

While iterating the parents you should check the cache at every level, this should provide the needed optimization.

@ai

This comment has been minimized.

Copy link
Member

@ai ai commented Oct 22, 2019

however reading the cache only looks at the current one

Oops, I forgot about it. I will try to fix it today.

@Knagis

This comment has been minimized.

Copy link
Author

@Knagis Knagis commented Oct 22, 2019

I will try the change and if it works, send a PR

@Knagis

This comment has been minimized.

Copy link
Author

@Knagis Knagis commented Oct 22, 2019

The PR improves the situation, now it is 60%, instead of 70%.

Still the resolve/sort part repeats on every file - perhaps there is something that can be also cached now that config file will properly be reused.

@ai

This comment has been minimized.

Copy link
Member

@ai ai commented Oct 22, 2019

A small optimization for sort browserslist/browserslist@84227e2

@ai

This comment has been minimized.

Copy link
Member

@ai ai commented Oct 22, 2019

Can you try this commit? I added cache for resolve and sort browserslist/browserslist@2fee1e4

@Knagis

This comment has been minimized.

Copy link
Author

@Knagis Knagis commented Oct 22, 2019

I will try it out tomorrow.

@Knagis

This comment has been minimized.

Copy link
Author

@Knagis Knagis commented Oct 23, 2019

Looks much better.
image

@ai

This comment has been minimized.

Copy link
Member

@ai ai commented Oct 24, 2019

Released in Browserslist 4.7.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.