Skip to content

Conversation

@mpsijm
Copy link
Contributor

@mpsijm mpsijm commented Jun 1, 2022

#136: Speed up execution time with caches

Description

This PR speeds up tailwindcss/enforces-shorthand and tailwindcss/classnames-order with officialSorting: true by introducing WeakMap caches 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-order with officialSorting: false, but I couldn't find anything that would significantly speed things up.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I've timed the tailwindcss/* rules using a minimal .eslintrc file on my code base (which I can't share, unfortunately).

$ TIMING=1 node --inspect ./node_modules/.bin/eslint .
$ time node --inspect ./node_modules/.bin/eslint .

Full details on the results of the timings can be found in the commit messages of 33d513a and 487121a.

Test Configuration:

  • OS + version: Arch Linux (last upgraded two weeks ago)
  • NPM version: 8.5.5
  • Node version: 16.15.0

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

mpsijm added 2 commits June 1, 2022 18:47
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
    }
  }
}
```
@mpsijm mpsijm changed the title #136: Speed up execution time #136: Speed up execution time with caches Jun 1, 2022
@francoismassart francoismassart merged commit 530438e into francoismassart:master Jun 8, 2022
@mpsijm mpsijm deleted the fix/issue-136-speed branch June 9, 2022 07:05
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.

2 participants