1856658 - Improve the performance of `setBreakpointPositions` action

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement)

Description

[Alexandre Poirot [:ochameau]]

In bug 1843454 profiler records:
https://share.firefox.dev/45i4Oph
We can see that we spend almost a second in setBreakpointPositions action.
This action is actually cloning and parsing a 200k array many times, while also going through many intermediate objects.
We can drastically optimize this by avoiding the use of Array's filter method.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1856658 - [devtools] Document breakable position retrieval and avoid instation of many Sets and Arrays.

This code is a bit hard to follow, so let's add some comments.

Also columns = [...new Set([...existing, ...columns])]; is probably very bad for performance.
It clones existing and columns into a third array, whose content is cloned into a Set,
which is ultimately cloned into the final array.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1856658 - [devtools] Drop unused options/search argument to getOriginalLocation(s).

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1856658 - [devtools] Better integrate SourceMapLoader into breakable positions computation.

We can avoid the creation of an handful of object creations by passing
the "breakpoint positions" objects directly to the Source Map Loader.
(avoid convertToList)

We can also tune SourceMapLoader.getOriginalLocations output to better
fit the requirements of the main thread.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1856658 - [devtools] Avoid duplicating the large breakpoints positions array when using function filtering methods.

The filterBySource was looping over the whole list once again
but worse, was duplicating the array once more.

Same for filterByUniqLocation.

Finally, once we removed these two last filtering methods... we could prevent the creation of the whole "positions"
intermediate object and craft directly what we aim to store in redux.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/caeaed53f213
[devtools] Document breakable position retrieval and avoid instation of many Sets and Arrays. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/8a22579ffeda
[devtools] Drop unused options/search argument to getOriginalLocation(s). r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/493f560d9831
[devtools] Better integrate SourceMapLoader into breakable positions computation. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/775a3be92f25
[devtools] Avoid duplicating the large breakpoints positions array when using function filtering methods. r=devtools-reviewers,nchevobbe

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/caeaed53f213
https://hg.mozilla.org/mozilla-central/rev/8a22579ffeda
https://hg.mozilla.org/mozilla-central/rev/493f560d9831
https://hg.mozilla.org/mozilla-central/rev/775a3be92f25

Comment 7

[Dennis Jackson]

Attachment: Bug 1856658 - Fix PSM test which relied on removed Verisign Cert. r=#nss-reviewers

Depends on D193535

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39ecb02fd8de
Fix PSM test which relied on removed Verisign Cert. r=nss-reviewers,bbeurdouche

Comment 9

[Sandor Molnar[:smolnar]]

Backed out for causing doc-generate failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/0af2bfddc084f01eb052a0bbd79ce7801981e9e5

Push with failures

Failure log -> Traceback (most recent call last):
[task 2023-11-15T17:30:58.524Z] File "/builds/worker/.mozbuild/srcdirs/gecko-8a5b87fe5d69/_virtualenvs/docs/lib/python3.8/site-packages/sphinx/ext/autodoc/importer.py", line 58, in import_module
[task 2023-11-15T17:30:58.524Z] return importlib.import_module(modname)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3bdf62c4b0fa
Fix PSM test which relied on removed Verisign Cert. r=nss-reviewers,bbeurdouche

Comment 11

[Atila Butkovits]

Backed out for refering the wrong bug.

Backout link: https://hg.mozilla.org/integration/autoland/rev/63cbca83011974bb18b3acd23b489f5577b83c3e