Skip to content

Commit

Permalink
#136: Speed up execution time with caches (#143)
Browse files Browse the repository at this point in the history
* #136: Speed up `tailwindcss/enforces-shorthand`

Using chrome:https://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
    }
  }
}
```

* #136: Speed up `tailwindcss/classnames-order` with `officialSorting: true`

Using chrome:https://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
    }
  }
}
```
  • Loading branch information
mpsijm authored Jun 8, 2022
1 parent f44f72f commit 530438e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
11 changes: 10 additions & 1 deletion lib/rules/classnames-order.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const createContextFallback = require('tailwindcss/lib/lib/setupContextUtils').c
// messageId will still be usable in tests.
const INVALID_CLASSNAMES_ORDER_MSG = 'Invalid Tailwind CSS classnames order';

const contextFallbackCache = new WeakMap();

module.exports = {
meta: {
docs: {
Expand Down Expand Up @@ -88,7 +90,14 @@ module.exports = {
const removeDuplicates = getOption(context, 'removeDuplicates');

const mergedConfig = customConfig.resolve(twConfig);
const contextFallback = officialSorting ? createContextFallback(mergedConfig) : null;
const contextFallback = officialSorting
? (
// Set the created contextFallback in the cache if it does not exist yet.
contextFallbackCache.has(mergedConfig)
? contextFallbackCache
: contextFallbackCache.set(mergedConfig, createContextFallback(mergedConfig))
).get(mergedConfig)
: null;

//----------------------------------------------------------------------
// Helpers
Expand Down
15 changes: 12 additions & 3 deletions lib/util/groupMethods.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,14 +261,23 @@ function generateOptions(propName, keys, config, isNegative = false) {
}
}

const cachedRegexes = new WeakMap();

/**
* Customize the regex based on config
*
* @param {String} re Regular expression
* @param {Object} config The merged Tailwind CSS config
* @returns {String} Patched version with config values and additinal parameters
* @returns {String} Patched version with config values and additional parameters
*/
function patchRegex(re, config) {
if (!cachedRegexes.has(config)) {
cachedRegexes.set(config, {});
}
const cache = cachedRegexes.get(config);
if (re in cache) {
return cache[re];
}
let patched = '\\!?';
// Prefix
if (config.prefix.length) {
Expand All @@ -281,7 +290,7 @@ function patchRegex(re, config) {
const resArray = [...res];
const props = resArray.map((arr) => arr[1]);
if (props.length === 0) {
return `${patched}(${replaced})`;
return cache[re] = `${patched}(${replaced})`;
}
// e.g. backgroundColor, letterSpacing, -margin...
props.forEach((prop) => {
Expand Down Expand Up @@ -336,7 +345,7 @@ function patchRegex(re, config) {
const opts = generateOptions(absoluteProp, keys, config, isNegative);
replaced = replaced.replace(token, opts);
});
return `${patched}(${replaced})`;
return cache[re] = `${patched}(${replaced})`;
}

/**
Expand Down

0 comments on commit 530438e

Please sign in to comment.