1942149 - Netmonitor extremely slow when switching panels with 1000+ request

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Description

[Julian Descottes [:jdescottes]]

STRs:

• accumulate 1000+ requests in netmonitor
• select another DevTools panel
• go back to netmonitor

ER: should be relatively fast
AR: on a powerful laptop, takes almost a minute for 10k requests

Might be the root cause for the netmonitor performance issues we have seen such as Bug 1880744.

The netmonitor is using the visibility-handler connect in most components which need to be connected to a store.
When the visibility of the panel changes the VisibilityHandlerComponent will call forceUpdate on each of those connected components.

In particular, 2 components rendered in each and every request item are connected via the visibility handler

• devtools/client/netmonitor/src/components/request-list/RequestListColumnFile.js
• devtools/client/netmonitor/src/components/request-list/RequestListColumnWaterfall.js

Which means that for a visibility change on 10k requests, we will call forceupdate on at least 20 000 components ...
There's not much JS activity in actual devtools code, but react simply takes a lot of time to attempt to render (in practice it mostly does nothing as not much actually needs to be updated).

I think in theory only the topmost panel component needs to be connected to the visibility handler, the rest can use the default react-redux connect?

Comment 1

[Julian Descottes [:jdescottes]]

Adding a dedicated test we can see a clear improvement by stopping to connect all components to the visibility handler: https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=b8a7c9b49a13257261ec3428cfb8591185f0b3f9&originalSignature=59287&newSignature=59287&framework=12&application=firefox&originalRevision=b2992102ae8345d3c73b59e5e4ab9b58b909e096&page=1&showOnlyImportant=1

However the downside is that the visibility handler also prevents rendering while the component is not visible, and that is still beneficial.
Maybe we need another connect method which will configure the HOC slightly differently.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1942149 - [devtools] Add netmonitor performance test for many requests in the list

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1942149 - [devtools] Only force updates on visibility change for the App netmonitor component

Depends on D234612

The VisibilityHandler can still be valuable for non root components, as it prevents background renders.
However forcing updates on all those components on VisibilityChange results in significant performance
issues when there are too many requests displayed.

This patch introduces a variant for visibility-handler-connect, which will only prevent background renders
without forcing update on visibility change.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1942149 - [devtools] Avoid spamming the logs during netmonitor perf tests

Depends on D234613

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1942149 - [devtools] Stop connecting components rendered for each request

Depends on D234789

Those two components are rendered for each request but are connected to the store to
extract state information which is usually global (or at least does not depend on the
request item).

This leads to a lot of time spent in react when there is a state update and many requests
are displayed.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c188dbbc1b18
[devtools] Add netmonitor performance test for many requests in the list r=devtools-reviewers,perftest-reviewers,fbilt,ochameau
https://hg.mozilla.org/integration/autoland/rev/db8f1d715259
[devtools] Avoid spamming the logs during netmonitor perf tests r=devtools-reviewers,perftest-reviewers,bomsy,kshampur
https://hg.mozilla.org/integration/autoland/rev/26b1d526e571
[devtools] Stop connecting components rendered for each request r=devtools-reviewers,ochameau

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/c188dbbc1b18
https://hg.mozilla.org/mozilla-central/rev/db8f1d715259
https://hg.mozilla.org/mozilla-central/rev/26b1d526e571

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: test_case.html

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: test_case.html

Comment 10

[Julian Descottes [:jdescottes]]

Attachment: test_case.html