1857214 - Optimize the parser worker computations

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement)

Description

[Alexandre Poirot [:ochameau]]

The parser worker does heavy computations based on Babel's output.
There is a few things we can tweak to avoid some allocations and speedup some code.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1857214 - [devtools] Avoid instantiating lots of intermediate arrays when computing the identifiers.

On bug 1843454 STR, symbols.identifiers is a 100M large array.
So all these intermediate array cause a serious amount of short-lived arrays.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1857214 - [devtools] Avoid creating duplicated identifiers.

We were creating them and later filtering them out.
Also getUniqueIdentifiers was duplicating a possibly 100M+ large array...

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1857214 - [devtools] Remove unused attributes and methods from SimplePath.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1857214 - [devtools] Avoid computing expression untill this is actually used.

Computing the expression would involve calling Babel's generate method on the AST node,
which is expansive. But we rarely read this attribute.

Comment 5

[Alexandre Poirot [:ochameau]]

This represents lots of patches... but with a quite small improvement on DAMP:
https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=8b23302336ab969a2d1c506bf633fa1287157536&originalSignature=4763542&newSignature=4763542&framework=12&application=firefox&originalRevision=43c06a822362f1cdcbd63b074ccabc96f4e85fd6&page=1&showOnlyConfident=1

But hopefully, this will reduce GC overhead, which isn't so well covered on DAMP.
For some reason, on DAMP, we are rarely blocked by a pending GC in the parser worker. Whereas IRL, this is very common and can delay very significantly variable preview.

Comment 6

[Alexandre Poirot [:ochameau]]

When opening the debugger against http://techno-barje.fr/fission/large-bundle/ and reloading the page once,
we can see significant different in GC.

Profile without any of these patches:
https://share.firefox.dev/46Q7Ged

Profile with all these patchs:
https://share.firefox.dev/3rSG09q

We move from 10s+ GC in the parser worker down to 7s.
The most important change is this 4.5s pause doing GC without the patches after we start reloading the page.
This delays the computation of the new original file AST.
This pause, with all these patches is down to 1.3s.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1857214 - [devtools] Remove some unused attributes from parser worker.

There might be more.

While doing that, document the objects being stored and their precise usage.

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1857214 - [devtools] Stop collecting imports to know if the module is a react module.

As imports are only used for knowing if that's a react module...
We could avoid a bunch of intermediate objects by computing right away if we have a react import.

Comment 9

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1857214 - [devtools] Stop collecting call expressions to know if the module is a react module.

As callExpressions are only used for knowing if that's a react module...
We could avoid a bunch of intermediate objects by computing right away if we have a react import.

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1857214 - [devtools] Don't store name for member expressions.

This attribute isn't used. Member expressions are only used by findBestMatchExpression.

Comment 11

[Alexandre Poirot [:ochameau]]

Actually, lazifying both expression for object expression and snippet brings even more improvements:
https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=e7bddc6bd6b0a5af5ddf9dff0d8e9fece7cde6ef&originalSignature=4763542&newSignature=4763542&framework=12&application=firefox&originalRevision=43c06a822362f1cdcbd63b074ccabc96f4e85fd6&page=1&showOnlyConfident=1

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03a7c78f3dd2
[devtools] Remove some unused attributes from parser worker. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/6d0ed4d665f2
[devtools] Stop collecting imports to know if the module is a react module. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/319d91a930d9
[devtools] Stop collecting call expressions to know if the module is a react module. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/0878e0610fb7
[devtools] Don't store name for member expressions. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/68eb34ac9527
[devtools] Avoid instantiating lots of intermediate arrays when computing the identifiers. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/b7ff0d21e05a
[devtools] Avoid creating duplicated identifiers. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/a86d62a27a74
[devtools] Remove unused attributes and methods from SimplePath. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/d1c21df521be
[devtools] Avoid computing expression untill this is actually used. r=devtools-reviewers,bomsy

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/03a7c78f3dd2
https://hg.mozilla.org/mozilla-central/rev/6d0ed4d665f2
https://hg.mozilla.org/mozilla-central/rev/319d91a930d9
https://hg.mozilla.org/mozilla-central/rev/0878e0610fb7
https://hg.mozilla.org/mozilla-central/rev/68eb34ac9527
https://hg.mozilla.org/mozilla-central/rev/b7ff0d21e05a
https://hg.mozilla.org/mozilla-central/rev/a86d62a27a74
https://hg.mozilla.org/mozilla-central/rev/d1c21df521be

Comment 14

[Julian Descottes [:jdescottes]]

== Change summary for alert #40006 (as of Thu, 19 Oct 2023 23:28:41 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
9%	damp custom.jsdebugger.open-large-minified-file.full-selection.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	5,737.60 -> 5,226.79
7%	damp custom.netmonitor.reload.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	388.51 -> 361.60
6%	damp custom.jsdebugger.open-large-minified-file.full-selection.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	3,895.37 -> 3,665.49
6%	damp custom.jsdebugger.open-large-minified-file.full-selection.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	3,863.30 -> 3,635.60
6%	damp custom.netmonitor.open.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	283.92 -> 268.12
...	...	...	...	...
4%	damp custom.jsdebugger.pretty-print.reload-and-pause.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	11,545.80 -> 11,124.38

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=40006