perf(rome_js_analyze): improve the performance of noUndeclaredVariables

[leops]

Summary

This PR improves the performance of the noUndeclaredVariables rule by deriving the Queryable trait on the SemanticServices type. This allows individual rules to query the SemanticModel as a whole and "manually" traverse it to emit a sequence of signals, instead of having to match on individual AST nodes. This is particularly useful for noUndeclaredVariables, since it's a lot cheaper to iterate on the list of all the unresolved references in the semantic model than to match on all the identifier nodes in the syntax tree.

Side note: while it may seem useful to use this new capability to query the semantic model for other lint rules, it is primarily intended as an advanced feature for optimizations and as such should only be used if you know what you're doing. I'm currently considering a way to implement Queryable on rome_js_semantic types such as Reference, Scope or Binding, and this would allow rules to directly express queries on semantic model concepts without have to implement the traversal and search logic themselves.

Test Plan

This is an infrastructure only change, it should not impact the results of the noUndeclaredVariables rule (or any other rules)

[netlify]

✅ Deploy Preview for rometools canceled.

Name	Link
🔨 Latest commit	ccd3cdc
🔍 Latest deploy log	https://app.netlify.com/sites/rometools/deploys/633d3e36f88263000811762e

[leops]

!bench_analyzer

[github-actions]

Playground for commit e4dd198

[github-actions]

Analyzer Benchmark Results

group                     main                                   pr
-----                     ----                                   --
analyzer/css.js           1.15      2.5±0.01ms     4.6 MB/sec    1.00      2.2±0.02ms     5.3 MB/sec
analyzer/index.js         1.11      7.1±0.03ms     4.6 MB/sec    1.00      6.4±0.07ms     5.1 MB/sec
analyzer/lint.ts          1.09      3.2±0.02ms    13.0 MB/sec    1.00      2.9±0.02ms    14.1 MB/sec
analyzer/parser.ts        1.13      8.4±0.07ms     5.8 MB/sec    1.00      7.4±0.06ms     6.6 MB/sec
analyzer/router.ts        1.14      6.2±0.10ms     9.9 MB/sec    1.00      5.4±0.02ms    11.3 MB/sec
analyzer/statement.ts     1.20      9.6±0.12ms     3.7 MB/sec    1.00      8.0±0.11ms     4.5 MB/sec
analyzer/typescript.ts    1.23     15.3±0.16ms     3.6 MB/sec    1.00     12.4±0.18ms     4.4 MB/sec

[leops]

@xunilrj As part of this change, I stored all the unresolved references in the semantic model and exposed a way to iterate on those. Does the implementation I came up with match how you would have envisioned this mechanism to work ?

[xunilrj]

@xunilrj As part of this change, I stored all the unresolved references in the semantic model and exposed a way to iterate on those. Does the implementation I came up with match how you would have envisioned this mechanism to work ?

Any reason it is a HashSet<...>? I would expect a Vec.
If I am raising more than one UnresolvedReference per range, I would consider it a bug.

I would also try to return a Reference instead of a SyntaxNode, to be on par with other functions returning references.

[xunilrj]

Nice! This opens a lot of room for performance improvements!

[leops]

Any reason it is a HashSet<...>? I would expect a Vec. If I am raising more than one UnresolvedReference per range, I would consider it a bug.

That was just in case to ensure each range could only be emitted once, but if that's already checked by the semantic event extractor I'll change it to a Vec

I would also try to return a Reference instead of a SyntaxNode, to be on par with other functions returning references.

I wasn't sure if it was "valid" to create a Reference object to represent an unresolved reference, for instance if the reference should use the existing read / write types or use a special "unresolved" reference type

[xunilrj]

I wasn't sure if it was "valid" to create a Reference object to represent an unresolved reference, for instance if the reference should use the existing read / write types or use a special "unresolved" reference type

We may need to create a new field read: bool for this event. The event extractor certainly knows if it is a read or a write, it is just not propagating this info.

[calibre-analytics]

Comparing perf(rome_js_analyze): improve the performance of noUndeclaredVariables Snapshot #5 to median since last deploy of rome.tools.

	LCP?	CLS?	TBT?
Overall Median across all pages and test profiles	2.09s  from 561ms	0.0 no change	168ms  from 46ms
Chrome Desktop Chrome Desktop • Cable	2.09s  from 575ms	0.0 no change	370ms  from 232ms
iPhone, 4G LTE iPhone 12 • 4G LTE	967ms  from 229ms	0.0 no change	6ms  from 12ms
Motorola Moto G Power, 3G connection Motorola Moto G Power • Regular 3G	14s  from 561ms	0.0 no change	168ms  from 46ms

1 page tested

 Home

Browser previews

Chrome Desktop	iPhone, 4G LTE	Motorola Moto G Power, 3G connection

Most significant changes

	Value	Budget
JS Parse & Compile Motorola Moto G Power, 3G connection	1.68s  from 31ms
Total JavaScript Size in Bytes Chrome Desktop	4.31 MB  from 86.8 KB
Total JavaScript Size in Bytes iPhone, 4G LTE	4.31 MB  from 86.8 KB
Total JavaScript Size in Bytes Motorola Moto G Power, 3G connection	4.31 MB  from 86.8 KB
JS Parse & Compile iPhone, 4G LTE	432ms  from 10ms

_{28 other significant changes: JS Parse & Compile on Chrome Desktop, First Contentful Paint on Motorola Moto G Power, 3G connection, Largest Contentful Paint on Motorola Moto G Power, 3G connection, Speed Index on Motorola Moto G Power, 3G connection, Total Page Size in Bytes on Chrome Desktop, Total Page Size in Bytes on iPhone, 4G LTE, Total Page Size in Bytes on Motorola Moto G Power, 3G connection, Number of Requests on Chrome Desktop, Number of Requests on iPhone, 4G LTE, Number of Requests on Motorola Moto G Power, 3G connection, Time to Interactive on Motorola Moto G Power, 3G connection, First Contentful Paint on Chrome Desktop, Time to Interactive on Chrome Desktop, First Contentful Paint on iPhone, 4G LTE, Speed Index on Chrome Desktop, Speed Index on iPhone, 4G LTE, Time to Interactive on iPhone, 4G LTE, Largest Contentful Paint on iPhone, 4G LTE, Total Blocking Time on Motorola Moto G Power, 3G connection, Largest Contentful Paint on Chrome Desktop, Total Image Size in Bytes on Chrome Desktop, Total Image Size in Bytes on iPhone, 4G LTE, Total Image Size in Bytes on Motorola Moto G Power, 3G connection, Total HTML Size in Bytes on Chrome Desktop, Total HTML Size in Bytes on iPhone, 4G LTE, Total HTML Size in Bytes on Motorola Moto G Power, 3G connection, Lighthouse Performance Score on Motorola Moto G Power, 3G connection, Lighthouse Performance Score on Chrome Desktop}

_{Calibre: Site dashboard | View this PR | Edit settings | View documentation}