state-snapshot-rule is surprisingly expensive #57
Description
Hello there!
I did some brief profiling with eslint, and found that a significant time spent linting our codebase was due to the state-snapshot-rule. Here is a single file example (note: the file has about 1300 lines of code):
time TIMING=1 pnpm exec eslint REDACTED-PATH/Playground.tsx
REDACTED-PATH/Playground.tsx
394:9 warning Better to just use proxy state valtio/state-snapshot-rule
422:5 warning Better to just use proxy state valtio/state-snapshot-rule
✖ 2 problems (0 errors, 2 warnings)
Rule | Time (ms) | Relative
:---------------------------------------|----------:|--------:
valtio/state-snapshot-rule | 51.368 | 32.2%
react/jsx-no-constructed-context-values | 15.081 | 9.5%
simple-import-sort/imports | 10.509 | 6.6%
react/no-direct-mutation-state | 10.086 | 6.3%
react/no-danger-with-children | 7.334 | 4.6%
react/no-typos | 6.383 | 4.0%
react/require-render-return | 6.233 | 3.9%
@typescript-eslint/naming-convention | 4.980 | 3.1%
react-hooks/exhaustive-deps | 3.521 | 2.2%
react/jsx-uses-react | 2.264 | 1.4%
TIMING=1 pnpm exec eslint REDACTED-PATH/Playground.tsx 21.42s user 4.65s system 123% cpu 21.173 total
As you can see, a single file takes 21s with the rule enabled. If I disable the rule, the entire file only takes ~15s:
Rule | Time (ms) | Relative
:---------------------------------------|----------:|--------:
react/jsx-no-constructed-context-values | 14.836 | 14.1%
react/no-direct-mutation-state | 10.372 | 9.9%
simple-import-sort/imports | 9.364 | 8.9%
@typescript-eslint/ban-types | 7.336 | 7.0%
react/no-typos | 6.632 | 6.3%
react/require-render-return | 6.350 | 6.0%
@typescript-eslint/naming-convention | 4.311 | 4.1%
react-hooks/exhaustive-deps | 3.657 | 3.5%
@typescript-eslint/no-empty-function | 2.524 | 2.4%
@typescript-eslint/no-redeclare | 2.368 | 2.3%
TIMING=1 pnpm exec eslint REDACTED-PATH/Playground.tsx 19.59s user 3.13s system 150% cpu 15.055 total
When multiplied across the codebase, the difference in timing is more noticeable. Before:
$ time TIMING=1 pnpm exec eslint .
Rule | Time (ms) | Relative
:---------------------------------------|----------:|--------:
valtio/state-snapshot-rule | 13505.425 | 40.9%
@typescript-eslint/no-redeclare | 3328.420 | 10.1%
react/no-direct-mutation-state | 2335.941 | 7.1%
react/jsx-no-constructed-context-values | 1800.343 | 5.5%
react/require-render-return | 1382.010 | 4.2%
react/no-typos | 1095.258 | 3.3%
@typescript-eslint/naming-convention | 1068.797 | 3.2%
simple-import-sort/imports | 1017.479 | 3.1%
react/jsx-uses-react | 774.067 | 2.3%
jsx-a11y/role-supports-aria-props | 476.845 | 1.4%
TIMING=1 pnpm exec eslint . 132.84s user 18.88s system 137% cpu 1:50.64 total
After disabling:
Rule | Time (ms) | Relative
:---------------------------------------|----------:|--------:
@typescript-eslint/no-redeclare | 3150.915 | 17.4%
react/no-direct-mutation-state | 1993.336 | 11.0%
react/jsx-no-constructed-context-values | 1739.310 | 9.6%
react/require-render-return | 1129.678 | 6.2%
simple-import-sort/imports | 972.616 | 5.4%
react/no-typos | 971.512 | 5.4%
@typescript-eslint/naming-convention | 968.518 | 5.4%
react/jsx-uses-react | 916.780 | 5.1%
jsx-a11y/role-supports-aria-props | 487.564 | 2.7%
no-restricted-imports | 428.382 | 2.4%
TIMING=1 pnpm exec eslint . 119.53s user 15.31s system 143% cpu 1:33.98 total
So, a 20% speedup after disabling this rule. Note that this is using eslint v7 (we're out of date), but I have also tried using v9 during an upgrade attempt and found very similar results. There is a chance this is something extremely specific to our codebase's use of types, but it's unclear to me how only state-snapshot-rule would be affected by that.
I apologize for not doing a deeper investigation, such as profiling/flamecharts, and just reporting! But I figured it was worth relaying our experience in case there was something obvious either we're doing wrong or that can be done to improve the performance of the rule. For now, we're going to disable the rule because it tends to also have false positives, which have reduced our developers' confidence (e.g. most most everyone was ignoring it! 🙃). Regardless, thank you for making this rule because it helped everyone learn 🚀