netmonitor janks the entire browser UI on visiblitychange events
Categories
(DevTools :: Netmonitor, defect, P2)
Tracking
(Not tracked)
People
(Reporter: florian, Unassigned)
References
(Depends on 1 open bug, Blocks 2 open bugs)
Details
(Keywords: perf, perf:responsiveness)
See this profile: https://share.firefox.dev/3WzOj7b
The profile shows that a "visibilitychange" DOM event handler in the chrome://devtools/content/netmonitor/index.html window blocked the parent process main thread for 46s.
My Firefox instance had a single window. The session has been running for multiple days. Devtools has never been used in the foreground tab, but it's likely that devtools has been open for the entire week-end in a background tab that updates periodically using XHR.
Given devtools wasn't visible, I don't think it should update anything on receiving a visibilitychange event (or maybe it shouldn't receive it at all).
Comment 1•6 months ago
|
||
In theory the visibility handler should not kick in when DevTools are used in a background tab, so we will check what could be wrong there. Then it's very likely that the netmonitor can be the culprit for the performance issue, because we have no cap for the number of requests in this panel. But in your case it shouldn't have been rendered at all.
Updated•6 months ago
|
Comment 2•6 months ago
|
||
Looking into it, I'm seeing a few things:
visibilitychange
is fired when we change the selected panel (so if Netmonitor was selected and you go to the settings panel)visibilitychange
is not fired when another tab is selectedvisibilitychange
is fired when a window is minimized/restored or when exiting screen saver, even when DevTools were opened in another tab than the selected one- the custom
connect
function we wrote is used for almost all connected components in the Netmonitor, which means that when thevisibilitychange
event handler fires, we force a lot of components to re-render
All those, combined with the fact there are no limits / virtualization, explains the issue Florian experienced.
I think the original intent of the VisibilityHandler
was to avoid hurting the other panels performance (Bug 1399242), but we may have missed the case where an "unselected" tab has DevTools opened.
Note that for the "regular" case of switching DevTools panels, we are manually dispatching the event: https://searchfox.org/mozilla-central/rev/7a8904165618818f73ab7fc692ace4a57ecd38c9/devtools/client/framework/toolbox.js#2794-2819 , we should check if this is still needed and/or if we should rely on other APIs instead
Reporter | ||
Comment 3•6 months ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
visibilitychange
is fired […] when exiting screen saver
This is what I encountered. Is it a platform bug, or is a docshell used by devtools incorrectly not set to inactive?
Comment 4•6 months ago
|
||
(In reply to Florian Quèze [:florian] from comment #3)
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
visibilitychange
is fired […] when exiting screen saverThis is what I encountered. Is it a platform bug, or is a docshell used by devtools incorrectly not set to inactive?
Not sure, I filed Bug 1897187 so we can get a DOM peer look into this
Description
•