-
-
Notifications
You must be signed in to change notification settings - Fork 107
#136: Speed up execution time with caches #143
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
Merged
francoismassart
merged 2 commits into
francoismassart:master
from
mpsijm:fix/issue-136-speed
Jun 8, 2022
Merged
#136: Speed up execution time with caches #143
francoismassart
merged 2 commits into
francoismassart:master
from
mpsijm:fix/issue-136-speed
Jun 8, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using chrome://inspect/, I found that the `patchRegex` function took a
lot of time to calculate. It looked like it was often called with the
same parameters, which I have optimized by adding a cache.
The cache uses a `WeakMap`, storing a plain string-to-string object for
every `config` object that is passed to `patchRegex`. I chose to use
`WeakMap` here because the Tailwind CSS config object may change over
time (see lib/util/customConfig.js), and this allows the garbage
collector to clean the cache when an old config is no longer used.
On my codebase (which I can't share, unfortunately), this greatly
reduces the run time of the `tailwindcss/enforces-shorthand` rule.
Details of the run time:
Command executed:
```
$ TIMING=1 node --inspect ./node_modules/.bin/eslint .
```
Before this commit:
```
Rule | Time (ms) | Relative
:----------------------------------------------|----------:|--------:
tailwindcss/enforces-shorthand | 2444.777 | 70.0%
tailwindcss/no-custom-classname | 487.850 | 14.0%
tailwindcss/no-contradicting-classname | 289.491 | 8.3%
tailwindcss/classnames-order | 229.961 | 6.6%
tailwindcss/migration-from-tailwind-2 | 20.250 | 0.6%
tailwindcss/enforces-negative-arbitrary-values | 17.669 | 0.5%
```
After this commit:
```
Rule | Time (ms) | Relative
:----------------------------------------------|----------:|--------:
tailwindcss/enforces-shorthand | 564.097 | 32.8%
tailwindcss/no-custom-classname | 531.374 | 30.9%
tailwindcss/no-contradicting-classname | 341.007 | 19.8%
tailwindcss/classnames-order | 238.749 | 13.9%
tailwindcss/enforces-negative-arbitrary-values | 22.539 | 1.3%
tailwindcss/migration-from-tailwind-2 | 20.349 | 1.2%
```
Note that the timings above are single runs, not averages, but repeated
runs show a similar pattern.
ESLint config used (which, for testing, I have adapted in such a way
that _only_ the `tailwindcss/*` rules are executed):
```
{
"env": {"browser": true, "es2020": true, "node": true},
"extends": ["plugin:tailwindcss/recommended"],
"overrides": [{"files": ["*.ts", "*.tsx"]}],
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaFeatures": {"jsx": true},
"sourceType": "module"
},
"plugins": ["react", "@typescript-eslint"],
"settings": {
"react": {"version": "detect"},
"tailwindcss": {
"cssFiles": ["src/**/*.css"],
"officialSorting": true
}
}
}
```
…ficialSorting: true`
Using chrome://inspect/, I found that the `createContext` function from
the Tailwind API took a lot of time to execute. It turned out that this
function was called more than only a few times, so I have optimized this
by adding a cache.
The cache uses a `WeakMap`, storing a `contextFallback` object for
every `config` object that is passed to `patchRegex`. I chose to use
`WeakMap` here because the Tailwind CSS config object may change over
time (see lib/util/customConfig.js), and this allows the garbage
collector to clean the cache when an old config is no longer used.
Interestingly enough, the time to execute the `create` function for an
ESLint rule is not included in the time reported by `TIMING=1`, as can
be seen in the details below. However, on my codebase (which I can't
share, unfortunately), this roughly halves the total runtime of the
`eslint .` command when using only `tailwindcss/*` rules.
Details of the run time:
Commands executed:
```
$ TIMING=1 node --inspect ./node_modules/.bin/eslint .
$ time node --inspect ./node_modules/.bin/eslint .
```
Before this commit:
```
Rule | Time (ms) | Relative
:----------------------------------------------|----------:|--------:
tailwindcss/no-custom-classname | 491.130 | 32.6%
tailwindcss/enforces-shorthand | 483.797 | 32.1%
tailwindcss/no-contradicting-classname | 293.046 | 19.4%
tailwindcss/classnames-order | 206.097 | 13.7%
tailwindcss/migration-from-tailwind-2 | 16.470 | 1.1%
tailwindcss/enforces-negative-arbitrary-values | 16.250 | 1.1%
```
After this commit:
```
Rule | Time (ms) | Relative
:----------------------------------------------|----------:|--------:
tailwindcss/no-custom-classname | 482.266 | 34.0%
tailwindcss/enforces-shorthand | 434.553 | 30.6%
tailwindcss/no-contradicting-classname | 276.010 | 19.5%
tailwindcss/classnames-order | 180.044 | 12.7%
tailwindcss/migration-from-tailwind-2 | 24.113 | 1.7%
tailwindcss/enforces-negative-arbitrary-values | 20.486 | 1.4%
```
Note that the timings above are single runs, not averages.
Repeated runs show that the _reported_ time for `classnames-order` does
not change significantly.
However, the output of `time` shows a different picture:
before this commit, the `wall` time is 19s and the `user` time is 30s;
after this commit, the `wall` time is 10s and the `user` time is 17s.
These timings are averaged over 5 runs.
ESLint config used (which, for testing, I have adapted in such a way
that _only_ the `tailwindcss/*` rules are executed):
```
{
"env": {"browser": true, "es2020": true, "node": true},
"extends": ["plugin:tailwindcss/recommended"],
"overrides": [{"files": ["*.ts", "*.tsx"]}],
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaFeatures": {"jsx": true},
"sourceType": "module"
},
"plugins": ["react", "@typescript-eslint"],
"settings": {
"react": {"version": "detect"},
"tailwindcss": {
"cssFiles": ["src/**/*.css"],
"officialSorting": true
}
}
}
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#136: Speed up execution time with caches
Description
This PR speeds up
tailwindcss/enforces-shorthandandtailwindcss/classnames-orderwithofficialSorting: trueby introducingWeakMapcaches to reduce duplicate calculations.Since I cannot test this on @ronny1020's code base, I'm not sure if #136 can be marked as fixed with this PR. In particular, I also tried finding low-hanging fruit to optimise
tailwindcss/classnames-orderwithofficialSorting: false, but I couldn't find anything that would significantly speed things up.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
I've timed the
tailwindcss/*rules using a minimal.eslintrcfile on my code base (which I can't share, unfortunately).Full details on the results of the timings can be found in the commit messages of 33d513a and 487121a.
Test Configuration:
Checklist:
I have made corresponding changes to the documentationI have added tests that prove my fix is effective or that my feature worksNew andexisting unit tests pass locally with my changesAny dependent changes have been merged and published in downstream modules