Closed Bug 1857214 Opened 7 months ago Closed 7 months ago

Optimize the parser worker computations

Categories

(DevTools :: Debugger, enhancement)

Product:
DevTools
Component:
Debugger
Type:
enhancement

Tracking

(firefox120 fixed)

Status:
RESOLVED FIXED
Milestone:
120 Branch
Tracking Flags:
Tracking Status
firefox120 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(8 files)

Bug 1857214 - [devtools] Avoid instantiating lots of intermediate arrays when computing the identifiers.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1857214 - [devtools] Avoid creating duplicated identifiers.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1857214 - [devtools] Remove unused attributes and methods from SimplePath.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1857214 - [devtools] Avoid computing expression untill this is actually used.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1857214 - [devtools] Remove some unused attributes from parser worker.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1857214 - [devtools] Stop collecting imports to know if the module is a react module.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1857214 - [devtools] Stop collecting call expressions to know if the module is a react module.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1857214 - [devtools] Don't store name for member expressions.
48 bytes, text/x-phabricator-request
Details | Review

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.

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

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

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

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.

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.

There might be more.

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

Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED

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.

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.

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

Pushed by apoirot@mozilla.com:
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

== 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

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: