1653779 - Firefox freezes and becomes unusable on react/typescript/eval-source-map project

Status: NEW

(DevTools :: Debugger, defect, P3)

Description

[Andrei Petcu]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0

Steps to reproduce:

I ran a project built with react/typescript/eval-source-map locally

Actual results:

Firefox became unusable. Pages load forever. Here is a profile https://share.firefox.dev/3eH4YMD

Expected results:

It should have been fast, just like in Chrome.

Comment 1

[Andrei Petcu]

I tried to create a project that replicates this issue. I failed.
I'm willing to do a remote desktop session with a Firefox DevTools developer to help figure out this mistery.

Comment 2

[Andrei Petcu]

Ohh.... and I can build Firefox locally so a dev can actually test stuff on my machine.
I really hate this issue. It makes using Firefox to other developers in our company a really hard sell.

Comment 3

[:Harald Kirschner :digitarald]

Logan, anything obvious from the profile?

Comment 4

[Andrei Petcu]

I can offer an about:memory dump if this can help.

My goal here would is be able to use Firefox in our project so any workaround or build change to our project would be welcome.

Comment 5

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

I think an about:memory dump could help indeed.
Also it would be useful to have some more context: when does that happen? Was Firefox open for some time or had you just started it?
From the profile we mostly see some GC markers (and especially GCMajor) and that clearly freezes the main process.
How much memory does your Firefox take at that moment? What amount of memory do you have on your system?

For our React-based app, we disabled hot-reloading and instead we reload pages manually (webpack still auto-builds in background), as we realized Devtools may keep too many references at each reload. There are a lot of other opened bugzilla bugs related to this problem.

Comment 6

[Andrei Petcu]

Attachment: before-memory-report.json.gz

before memory dump

Comment 7

[Andrei Petcu]

Attachment: after-memory-report.json.gz

after memory dump

Comment 8

[Andrei Petcu]

Attachment: after.png

Comment 9

[Andrei Petcu]

Attachment: before.png

Comment 10

[Andrei Petcu]

I added a before and after about:memory. I ran the GC before taking the second one + minimise memory usage.
Firefox was just started.
It takes a bit more than 1G but it keeps going up (see after.png).
It usually takes a few hundreds of megs of ram (see before.png).
It's first page load that takes forever. I did not change any source code so hot reloading does not kick in.
If I open the page without devtools it takes a normal amount of time.

Can I help with any other info? :)
Thank you for looking into this.

Comment 11

[Andrei Petcu]

My machine has 32G of ram and I was using 9G when I made the print screens.

Comment 12

[Logan Smyth [:loganfsmyth]]

Looks like this is another case of the source file list processing causing things to grind to a halt.

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the report!

We care about performance, but there is currently none who could work on this now or in the next cycle. The right meta bug is properly set here so, let's just change the priority.

Honza

Comment 14

[Andrei Petcu]

I stopped using Firefox for js debugging on this project which is my main work project :(
It's still my default browser and I use it for everything else :)

Comment 15

[Andrei Petcu]

One of the few Firefox users in my team switched to Chrome by default because of this bug. At this point I'm happy with any workaround. I'll try to tweak the source maps maybe other source maps implementations may help with this issue.
I think we are loosing a lot of devs with this bug. Typescript + React is a really popular combination. It's sad that the JS engine is becoming faster and faster yet the perceived performance is bad because of this devtools issues. Firefox feels like the pre e10s builds now.

Comment 16

[mattheww]

Attachment: WIP: Bug 1653779 - Optimize source tree rendering and wait for it to complete

Comment 17

[mattheww]

I was able to duplicate this (I think) by using Create React App and adding a wildly excessive number of source files (around 3000): https://gitlab.com/wartmanm/debugger-examples/-/tree/master/eval-source-map
I think the issue is that, while adding new sources is debounced, there is no backpressure from rendering them. As the time taken to render passes 100ms, the debugger starts thrashing, adding 1 source, rendering, adding 1 source, etc.
I was able to get the load time down from 80 seconds: https://share.firefox.dev/3sr6aKL to around 6 seconds: https://share.firefox.dev/3akfycY by waiting for rendering to complete before starting the debounce timer, and by micro-optimizing some of the redux reducers. Chrome takes about the same amount of time, so while there is room for further improvement, this seems like a good start.

Comment 18

[Andrei Petcu]

sounds great! thank you!

Comment 19

[mattheww]

Attachment: WIP: Bug 1653779 - Only attach one message listener to devtools workers

Previously, a separate listener was added for every message, and all of them
were called for every response.

Comment 20

[mattheww]

Attachment: WIP: Bug 1653779 - Remove caching from queryAllDisplayedSources

It had a hit rate of only 10-12%, and didn't seem to be worth the added
overhead.

Depends on D115312

Comment 21

[mattheww]

Attachment: WIP: Bug 1653779 - Use plainUrl in getDisplayedSources

This makes use of state.sources.plainUrls in getDisplayedSources, makes
getPlainUrl and stripQuery consistent, and sets up for including plain urls
directly in sources.

Depends on D115313

Comment 22

[mattheww]

Attachment: WIP: Bug 1653779 - Store plainUrl on sources

This eagerly computes sources' display URLs as they are added. They were
memoized, but the memoization map lookup itself took time.

Depends on D115316

Comment 23

[mattheww]

Attachment: Bug 1653779 - Lazily create parent map

parentMap appears to only be used when pressing the left arrow key to navigate
to the parent folder in the source tree. If this is too slow it could be
replaced with a traverseTree search.

Depends on D115317

Comment 24

[mattheww]

Attachment: Bug 1653779 - Miscellaneous micro-optimizations

These aren't big changes, or dramatic speedups, but there's no reason not to do
them.

Depends on D115318

Comment 25

[Nicolas Chevobbe [:nchevobbe]]

Hello mattheww,

I'm seeing you uploaded several smaller patches, that's great :)
I see they are tagged as WIP, are they ready for review? or are you blocked on something?

Comment 26

[mattheww]

They should be ready, I just forgot to add a reviewer. Thank you for the reminder!
I also posted a question on the original patch, I'm not sure you saw it? I was wondering if introducing cooldown() should take place in one bug or two, and if it'd be better to put it someplace like newGeneratedSources rather than right in resource-watcher.

Comment 27

[Phabricator Automation]

Comment on attachment 9222317 [details]
WIP: Bug 1653779 - Only attach one message listener to devtools workers

Revision D115312 was moved to bug 1714082. Setting attachment 9222317 [details] to obsolete.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5428b3d0a08
Lazily create parent map r=nchevobbe,bomsy

Comment 30

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/d5428b3d0a08

Comment 31

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:bomsy, maybe it's time to close this bug?
For more information, please visit auto_nag documentation.

Comment 32

[Hubert Boma Manilla (:bomsy)]

Lets still leave this open to investigate.

Comment 33

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a1c845899fe
Miscellaneous micro-optimizations r=nchevobbe,devtools-reviewers,ochameau

Comment 35

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2a1c845899fe

Comment 36

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 37

[Alexandre Poirot [:ochameau] [PTO back August 21]]

matthew, I couldn't manage to build'n run your test app from comment 17.
We have been fixing lots of performance issues all around the debugger, I'm curious if you still have example of scenarios where the performance is still an issue?

Comment 38

[Alexandre Poirot [:ochameau] [PTO back August 21]]

Andrei, I have the same question, do you still have an example of significant performance trouble with Firefox debugger?

Comment 39

[mattheww]

Wow, my example completely bitrotted. I've updated it, added the build output to the repository, and also added another example using a fabricated source map. I'm seeing good performance even with way more files than I had originally. I did notice a separate issue where the debugger would jump off of the files I selected: screen recording. Seems like a race condition? I'll take a look for an existing bug.

Comment 40

[Alexandre Poirot [:ochameau] [PTO back August 21]]

(In reply to mattheww from comment #39)

Wow, my example completely bitrotted. I've updated it, added the build output to the repository, and also added another example using a fabricated source map. I'm seeing good performance even with way more files than I had originally. I did notice a separate issue where the debugger would jump off of the files I selected: screen recording. Seems like a race condition? I'll take a look for an existing bug.

Thanks a lot for getting back to this many years old issue!

The unexpected jump seems to be a regression of my work to polish support for sourcemap...
It behaves correctly if you enable the "show and open original location by default" in the new source map menu visible in the code editor's footer. (it should be displaying "bundle file" when selecting main.js).
I'll open a bug to address this. Selecting from the source tree should force selecting the original source regardless of this setting.

Comment 41

[Alexandre Poirot [:ochameau] [PTO back August 21]]

Matthew test case may be a bit extreme, but there is still room for improvement.
https://share.firefox.dev/40InQFy

• 12s of computations in the source map worker
  • 11s processing URLs :// Bug 1829610, which nchevobbe is actively looking into, should bring some nice improvement.
• 5s jank when processing the sources in the UI
  • 3s+ in source tree reducer (searchfox).

Comment 42

[Alexandre Poirot [:ochameau] [PTO back August 21]]

I opened bug 1976551 for the source selection issue.