Reduce memory allocation in rules and utils

[willheslam]

Hello! eslint-plugin-react is an amazing plugin - it catches so many problems that I consider it indispensable :)

Recently on a large-ish project I noticed some of the rules running relatively slowly, so I decided to see if I could speed them up. In the end I was able to find quite a few opportunities to do less work, mainly by avoiding memory allocations.

I'd really appreciate any feedback on better ways to benchmark the plugin, and how safe these optimisations are. They pass all the tests, though I appreciate it's not always possible to be 100% confident changes are correct, especially given the large number of changes here - luckily most of them are relatively localised. I've tried to avoid changing the existing logic as much as possible, though sometimes speed has been picked over readability - I'd appreciate any thoughts on that too. I've tried to make sure the changes will work on node 4, but that probably needs more thorough testing.

There's a list of the changes summarised at the bottom of this message. Thanks!

---

Benchmark results:

The project I benchmarked this against using cloc registers as about 550 files, with about 50k lines of code in .tsx files all containing React components. The changes knock off about 70 - 100ms off the slowest rules, roughly 30% faster, with a sample result here:

Rule                                       | Time (ms) | Relative
:------------------------------------------|----------:|--------:
react/jsx-indent                           |   365.723 |     5.8%
react/no-deprecated                        |   296.861 |     4.7%
react/boolean-prop-naming                  |   286.611 |     4.6%
react/destructuring-assignment             |   283.235 |     4.5%
react/no-unstable-nested-components        |   172.198 |     2.7%
react/no-access-state-in-setstate          |   169.871 |     2.7%
react/jsx-no-bind                          |   161.568 |     2.6%
react/no-direct-mutation-state             |   155.857 |     2.5%
react/prefer-stateless-function            |   150.361 |     2.4%
react/display-name                         |   147.024 |     2.3%
react/no-string-refs                       |   144.832 |     2.3%
react/jsx-sort-props                       |   142.020 |     2.3%
react/no-typos                             |   141.018 |     2.2%
react/default-props-match-prop-types       |   140.545 |     2.2%
react/void-dom-elements-no-children        |   139.404 |     2.2%

to

Rule                                       | Time (ms) | Relative
:------------------------------------------|----------:|--------:
react/jsx-indent                           |   264.386 |     6.4%
react/boolean-prop-naming                  |   176.753 |     4.3%
react/destructuring-assignment             |   146.563 |     3.6%
react/jsx-sort-props                       |   142.513 |     3.5%
react/no-access-state-in-setstate          |   117.366 |     2.8%
react/no-deprecated                        |   109.879 |     2.7%
react/no-direct-mutation-state             |   107.110 |     2.6%
react/void-dom-elements-no-children        |    98.467 |     2.4%
react/display-name                         |    97.823 |     2.4%
react/no-unstable-nested-components        |    95.287 |     2.3%
react/jsx-no-bind                          |    94.966 |     2.3%
react/default-props-match-prop-types       |    92.893 |     2.3%
react/sort-comp                            |    91.416 |     2.2%
react/prefer-stateless-function            |    86.666 |     2.1%
react/no-typos                             |    84.784 |     2.1%

I used the following .eslintrc.js:

module.exports = {
  root: true,
  parser: "@typescript-eslint/parser",
  settings: {
    react: {
      pragma: "React",
      fragment: "Fragment",
      version: "detect",
    },
  },
  extends: ["plugin:react/all"],
}

with the versions:

    "eslint": "^7.25.0",
    "eslint-plugin-react": "^7.23.2",
    "@typescript-eslint/eslint-plugin": "^4.22.0",
    "@typescript-eslint/parser": "^4.22.0",

across the .tsx files in the project, 200 times by running:

$ script timings
$ for i in {1..200}; do TIMING=100 npx eslint . --ext tsx -o eslintlog; done

for both 7.23.2 and then with the lib directory swapped in from this branch.

I then averaged all 200 runs in this google sheet: https://docs.google.com/spreadsheets/d/1Y5QZ_tsAyXrzgw5Rre21Gmzl95jT9irZmxfwSeF1oI0/edit?usp=sharing

---

Optimisations:

• Avoid allocating empty objects or arrays when possible
• Combine filter -> forEach chains
• Replace Object.keys with values/entries where possible
• Extract deeply nested functions to avoid reallocation
• Push expensive operations below guards / early returns
• Promote shared or mostly-constant conditionals
• Swap compound boolean expressions to proriotise cheap short circuiting
• Avoid dynamically generating RegExps when possible
• Lazily initialise and cache complex objects and necessarily dynamic RegExps
• Move expensive and shared calculations to higher scopes
• Replace string splits and trims with RegExps
• Avoid redundant array concatenations
• Split expensive conditional checks across multiple guards
• Move expensive booleans into conditional expressions to exploit short circuiting
• Swap out filters for finds where possible
• Turn arrays into objects/map/dictionary lookups where possible
• Swap array function orders where safe and efficient
• Cache string transformations

[codecov-commenter]

Codecov Report

Merging #2976 (232d6a1) into master (1185b37) will increase coverage by 0.00%. The diff coverage is 98.65%.

@@           Coverage Diff           @@
##           master    #2976   +/-   ##
=======================================
  Coverage   97.59%   97.59%           
=======================================
  Files         110      110           
  Lines        7269     7368   +99     
  Branches     2651     2695   +44     
=======================================
+ Hits         7094     7191   +97     
- Misses        175      177    +2     

Impacted Files	Coverage Δ
lib/rules/jsx-indent.js	97.29% <87.50%> (-0.04%)	:arrow_down:
lib/rules/void-dom-elements-no-children.js	95.65% <90.00%> (+0.19%)	:arrow_up:
lib/rules/no-unknown-property.js	96.49% <90.90%> (-0.07%)	:arrow_down:
lib/rules/no-unused-prop-types.js	95.83% <94.44%> (+2.08%)	:arrow_up:
lib/util/version.js	96.96% <96.00%> (-0.79%)	:arrow_down:
lib/util/Components.js	98.56% <98.70%> (-0.31%)	:arrow_down:
lib/rules/boolean-prop-naming.js	99.28% <100.00%> (+0.01%)	:arrow_up:
lib/rules/default-props-match-prop-types.js	100.00% <100.00%> (ø)
lib/rules/destructuring-assignment.js	98.86% <100.00%> (ø)
lib/rules/display-name.js	98.16% <100.00%> (+0.05%)	:arrow_up:
... and 36 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1185b37...232d6a1. Read the comment docs.

[willheslam]

It might be easier to review if this was split up into a separate commit for each conceptual kind of change - since some of them are no-brainers (like the Object.keys -> values) and others are more of a tradeoff between clarity and performance.

Thanks for suggesting this - I've now split this PR into 13 commits, roughly across the kinds of changes made, or, if hard to disentangle, across the individual modules that have been modified.

[ljharb]

@willheslam are you interested in continuing this effort? It seems like there's a number of comments to address, and a few potential PRs to land separately, plus this needs a rebase.

[willheslam]

@ljharb Sorry for disappearing on this, work + life got in the way and I ran out of time.

Yeah - I've got some time this week so I'll take a look at the changes I've made to address the comments, changes pending, and what the rebase will entail.

I appreciate your following up on this!