[Refactor] variableUtil: Avoid creating a single flat variable scope for each lookup

[DanielRosenwasser]

A few weeks ago (for reasons), I ended up profiling the time spent linting on the Sentry repository. One of the things I noticed was that a significant amount of time was being spent on variablesInScope and similar operations from the lint rules from the eslint-plugin-react repository.

The problem with most uses of variablesInScope is that this function creates a single flat scope out of all variables walking up the spine of a tree, and call-sites subsequently use that to look up the variables. This is a problem for a few reasons:

1. Intermediate arrays is thrown away on every parent scope encountered. In fact, several intermediate arrays may be created and thrown away at every scope as well if there are child scopes.
2. The final array is thrown away to construct the reversed array to provide correct ordering in lookup.
3. These calls are done frequently.
4. Many of the upper scopes never need to be consulted in the first place, so walking up all the scopes and creating one big array is possibly wasteful.

There may be more going on here (possibly the size of the global scope - I didn't spend much time looking into that). But at the very least, there's a lot of work to be avoided.

So this change swaps to a different lookup method that just walks up the scopes to find the variable. I've tried to introduce this change in a way that is the least disruptive to the codebase so it becomes a little easier to review.

Here's the time spent linting before this change:

Rule	Time (ms)	Relative
react/jsx-fragments ⬅️	58443.993	43.8%
import/no-duplicates	18127.050	13.6%
react/no-direct-mutation-state ⬅️	8056.287	6.0%
@typescript-eslint/naming-convention	6710.821	5.0%
react/no-typos ⬅️	5206.746	3.9%
react/sort-comp ⬅️	4908.256	3.7%
react/require-render-return ⬅️	4751.416	3.6%
react/function-component-definition ⬅️	4631.519	3.5%
no-lookahead-lookbehind-regexp/no-lookahead-lookbehind-regexp	3685.390	2.8%
@typescript-eslint/await-thenable	3464.244	2.6%

Here's the time spent linting after this change:

Rule	Time (ms)	Relative
import/no-duplicates	18185.205	29.2%
@typescript-eslint/naming-convention	7785.485	12.5%
react/no-direct-mutation-state ⬅️	5025.885	8.1%
no-lookahead-lookbehind-regexp/no-lookahead-lookbehind-regexp	3849.306	6.2%
@typescript-eslint/await-thenable	3428.845	5.5%
react/sort-comp ⬅️	2118.319	3.4%
react/no-typos ⬅️	2112.039	3.4%
simple-import-sort/imports	1974.583	3.2%
react/require-render-return ⬅️	1789.843	2.9%
react/function-component-definition ⬅️	1739.382	2.8%

Notice that react/jsx-fragments no longer shows up in the top 10 rules, and most other react/ rules have dropped by a decent chunk of time as well.

On the machine I tested this on, over 70 seconds are saved out of 134 seconds of pure lint time (not excluding the time spent loading up the entire program).

[ljharb]

awesome, thanks! this looks great.

[socket-security]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher

🚮 Removed packages: npm/[email protected]

View full report↗︎