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

Performance: Cache missingSupport and refactor Detector #182

Merged
merged 4 commits into from
Aug 23, 2024

Conversation

cmrn
Copy link
Contributor

@cmrn cmrn commented Mar 6, 2024

This PR improves the performance of doiuse in two ways, making it run 3-10 times faster:

  1. Cache the result of BrowserSelection.missingSupport(), since compiling the list of missing features is an expensive operation. This improves performance when the postcss plugin is run over multiple CSS files, like when linting an entire codebase.
  2. Refactor Detector to flatten and normalise all feature checks during initialization, rather than using expensive Object.entries operations to parse the rules at every node in the syntax tree.

Benchmarks (from master...RJWadley:doiuse:benchmark-2):

run time diff
Before changes 5.3s -
Cache missingSupport 4.8s -0.5s
Refactor of Detector 2.2s -3.1s
After both changes 1.7s -3.6s

Other benchmarks:

benchmark before after
postcss/benchmark 233ms 44ms
stylelint's benchmark 6s 1.5s
private repo: ~300 CSS files 50s 5s
private repo: ~8300 CSS files 8m 50s 1m 30s

@cmrn cmrn changed the title Performance: Cache missingSupport and refactor Detector Performance: Cache missingSupport and refactor Detector Mar 6, 2024
Copy link
Collaborator

@clshortfuse clshortfuse left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It all looks good. Just left some comments in case some improvements can be made.

lib/Detector.js Outdated Show resolved Hide resolved
lib/BrowserSelection.js Show resolved Hide resolved
lib/Detector.js Outdated Show resolved Hide resolved
lib/Detector.js Outdated Show resolved Hide resolved
@cmrn
Copy link
Contributor Author

cmrn commented Jun 17, 2024

hi @clshortfuse, just a friendly bump on this PR :)

@clshortfuse clshortfuse merged commit 85d805d into anandthakker:master Aug 23, 2024
0 of 3 checks passed
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 this pull request may close these issues.

3 participants