composites happen at 60Hz until the browser window is closed after showing an arrowpanel
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
Performance | P1 |
People
(Reporter: florian, Assigned: hiro)
References
(Depends on 1 open bug, Blocks 2 open bugs)
Details
(Keywords: perf:resource-use, power)
Attachments
(1 file, 1 obsolete file)
I found steps to reproduce for a case that causes composites to happen at 60Hz until the browser window is closed.
- Preparation: in a first browser window, start a download (eg. http://test-debit.free.fr/10485760.rnd), let it progress for a few seconds, then cancel it. This makes the downloads toolbar icon appear on all browser windows.
- Open a new browser window, and close the first browser window.
- Start the profiler.
- Click the downloads toolbar icon, this makes the downloads panel appear. Wait for a few seconds.
- Dismiss the downloads panel. Wait for a few seconds.
- (Optional) Open a new browser window, and close the browser window that was opened at step 2.
- Capture the profile.
I have been able to reproduce consistently on the current Nightly on 2 Windows machines. On my fast-ish development laptop (https://share.firefox.dev/3nPqRkF) and on the Quantum reference laptop (https://share.firefox.dev/30Vk6VH). I have not been able to reproduce on Mac (and I have not tried on Linux yet).
In the profiles, when we composite at 60Hz for no obvious reason, the Screenshots track of the downloads panel is black. Reopening the downloads panel makes the composition rate go down to normal.
After step 2, composition happens normally. After step 4 it still happens normally. After step 5 (hiding the downloads panel), it happens at 60Hz. After step 6 it goes back to normal.
Emilio, Markus, could you please forward this needinfo to someone who can figure this out? I'm not sure if this is in cross platform code or if it's Windows specific.
Reporter | ||
Comment 1•6 months ago
|
||
(In reply to Florian Quèze [:florian] from comment #0)
I have not been able to reproduce on Mac (and I have not tried on Linux yet).
On Linux I can't reproduce the 60Hz composite. The one part of the behavior that exists on Linux too is that the panel has its own Screenshots track that ends only when closing the browser window it was attached to. But there's no black screenshots on that track. https://share.firefox.dev/3cMamj0
Reporter | ||
Updated•6 months ago
|
Comment 2•6 months ago
|
||
I couldn't repro either on Linux... Maybe Sotaro knows what might cause this on Windows? Maybe windows keeps sending us paint events? Maybe his work on occlusion helps popups as well, unclear.
Comment 3•6 months ago
|
||
I currently don't have a Windows machine to reproduce.
Comment 5•6 months ago
•
|
||
:florian, can you check FPS with debug overlay? It could be checked with the following prefs
- gfx.webrender.debug.profiler-ui:FPS
- gfx.webrender.debug.profiler:true
I saw the a lot of "Paint" markers in profiler. But FPS of debug overlay was very low.
It seems like handling of profiler marker is not good. "Paint" marker id set even when rendering does not happen. Rendering happens only when aRender==true.
https://searchfox.org/mozilla-central/rev/7fe9421af35256a95acc4620e9e0b76df7867287/gfx/webrender_bindings/RenderThread.cpp#489
Comment 6•6 months ago
|
||
:mstange, can you comment to "Paint" marker of comment 5?
Reporter | ||
Comment 7•6 months ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #5)
:florian, can you check FPS with debug overlay? It could be checked with the following prefs
- gfx.webrender.debug.profiler-ui:FPS
- gfx.webrender.debug.profiler:true
I saw the a lot of "Paint" markers in profiler. But FPS of debug overlay was very low.
The debug overlay disappears at the same time as the panel when I close the panel, so I can't see what it would show when the bug occurs.
It seems like handling of profiler marker is not good. "Paint" marker id set even when rendering does not happen. Rendering happens only when aRender==true.
https://searchfox.org/mozilla-central/rev/7fe9421af35256a95acc4620e9e0b76df7867287/gfx/webrender_bindings/RenderThread.cpp#489
I don't know if that marker is accurate or misleading. The main problem in this bug is that activity is happening at 60Hz (eg. the PVsyncBridge::Msg_NotifyVsync IPCs are also a problem).
Comment 8•6 months ago
|
||
FYI the criticality of this issue will increase as we ship bug 1733587 since the panel will now open automatically for new downloads.
Comment 9•6 months ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #6)
:mstange, can you comment to "Paint" marker of comment 5?
I agree it could be better - we should emit a "Renderer Update #WIN" marker instead of a "Composite" marker if aRender
is false
. However, as Florian said, the real issue still stands: We are waking up at 60Hz. We should stop listening for vsync and let the CPU sleep.
Comment 10•6 months ago
|
||
The Downloads panel has fade-out effect when it closes: https://searchfox.org/mozilla-central/rev/65d4d3399afa79c8de5a0cc11752d2ba7c31edc1/toolkit/content/xul.css#319-326
This opacity animation runs on the compositor. It is possible that we never stop the compositor-side animation if the panel is closed, maybe because we never get to an aRender == true
composite when the animation is stopped.
Hiro, could you take a look?
Comment 11•6 months ago
•
|
||
Looks like I filed this four years ago, in bug 1413763, and then lost track of it. To answer Hiro's question from back then:
(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
How do we hide the panel?
If we do hide by setting visiblity:hidden, the transform animation keep
running.
The panel is hidden by making its nsView hidden and by closing the view's nsIWidget. The panel widget has its own compositor.
Comment 13•6 months ago
|
||
It is possible that making the hidden panel display:none would stop the compositor animation. This is currently being investigated in bug 1714846.
Reporter | ||
Comment 14•6 months ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
Florian, can you check if this is a regression?
I can reproduce on the 2020-05-22 build, which is the oldest build where the profiler works.
Assignee | ||
Comment 15•6 months ago
|
||
It looks like the animation (in fact it's two transtions) in question has a finite duration (0.18s), so it sounds odd that the animation keeps running on the compositor.
Assignee | ||
Comment 16•6 months ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #15)
It looks like the animation (in fact it's two transtions) in question has a finite duration (0.18s), so it sounds odd that the animation keeps running on the compositor.
Never mind about this comment. I did forget that we use fill-mode: both for transitions to avoid glitches on the compositor, thus it keeps running until an explicit transaction which makes the transitions stop happens.
Comment 17•5 months ago
|
||
The severity field is not set for this bug.
:jimm, could you have a look please?
For more information, please visit auto_nag documentation.
![]() |
||
Updated•5 months ago
|
![]() |
||
Updated•5 months ago
|
Comment 18•5 months ago
|
||
I think this is fixed by bug 1714846 - Florian, can you confirm? I'm not particularly convinced because I don't see any composite markers at all, so I assume the steps in comment 0 miss, say, which profiler preset you're meant to use or where to look for the markers? https://share.firefox.dev/3EVl5nu is my profile. I did already figure out I had to turn off screenshots because otherwise those trigger markers for the screenshots that seemed like they'd interfere with actually determining what was going on here... but I've no idea if there's anything else I'm missing.
Reporter | ||
Comment 19•5 months ago
|
||
(In reply to :Gijs (he/him) from comment #18)
I think this is fixed by bug 1714846 - Florian, can you confirm? I'm not particularly convinced because I don't see any composite markers at all, so I assume the steps in comment 0 miss, say, which profiler preset you're meant to use or where to look for the markers? https://share.firefox.dev/3EVl5nu is my profile.
I guess I forgot to say "look in the Renderer or compositor thread". Your profile has composites until you close the browser window (the parent process has a DOMWindowClose event when the composite markers stop) https://share.firefox.dev/3s2NcO4
I did already figure out I had to turn off screenshots because otherwise those trigger markers for the screenshots that seemed like they'd interfere with actually determining what was going on here...
I'm curious what made you think this. We create a screenshot marker for every composite, and won't trigger any if there's no composite happening.
Comment 20•5 months ago
•
|
||
(In reply to Florian Quèze [:florian] from comment #19)
(In reply to :Gijs (he/him) from comment #18)
I think this is fixed by bug 1714846 - Florian, can you confirm? I'm not particularly convinced because I don't see any composite markers at all, so I assume the steps in comment 0 miss, say, which profiler preset you're meant to use or where to look for the markers? https://share.firefox.dev/3EVl5nu is my profile.
I guess I forgot to say "look in the Renderer or compositor thread". Your profile has composites until you close the browser window (the parent process has a DOMWindowClose event when the composite markers stop) https://share.firefox.dev/3s2NcO4
So does that mean this is fixed or did we expect the patch to mean that the composites stop when the panel is closed even while that window remains open? If the latter, who is in a good position to investigate why that doesn't happen? If the former, should we close this bug?
I did already figure out I had to turn off screenshots because otherwise those trigger markers for the screenshots that seemed like they'd interfere with actually determining what was going on here...
I'm curious what made you think this. We create a screenshot marker for every composite, and won't trigger any if there's no composite happening.
The composite markers for screenshots seem to appear even when there is no composite marker on the compositor thread - compare https://share.firefox.dev/3IOceqd and https://share.firefox.dev/3pYsTid (different views on the same profile (with screenshots this time) . Perhaps that's a separate bug?
Comment 21•5 months ago
•
|
||
(In reply to :Gijs (he/him) from comment #20)
The composite markers for screenshots seem to appear even when there is no composite marker on the compositor thread - compare https://share.firefox.dev/3IOceqd and https://share.firefox.dev/3pYsTid (different views on the same profile (with screenshots this time) . Perhaps that's a separate bug?
Note that you can Cmd-click threads and get an integrated marker chart of multiple threads: https://share.firefox.dev/3p0PlYM (I also applied a search filter for "composit")
The screenshot markers span the distance between two consecutive composites of the same window.
Comment 22•5 months ago
|
||
Ah, and in this particular case, it looks like we're getting "Composite" markers for the hidden panel, but no updated "CompositorScreenshot" markers - instead, the panel's synthesized "CompositorScreenshot" marker spans the full duration of 4.6 seconds until the parent window is closed. So it seems like we're triggering a composite but then canceling compositing somewhere in the pipeline before we capture a new screenshot. Likely because something notices that the window is closed. But the frequent "Composite" and "CompositeToTarget" markers indicate that we still wake up the CPU. And that's the part that this bug is about.
Reporter | ||
Comment 23•5 months ago
|
||
(In reply to :Gijs (he/him) from comment #20)
So does that mean this is fixed or did we expect the patch to mean that the composites stop when the panel is closed even while that window remains open? If the latter, who is in a good position to investigate why that doesn't happen?
We expect composites for the panel to stop when the panel is closed. Given the previous comments (especially comment 16), I would say Hiro would be in a good position to keep investigating.
Comment 24•5 months ago
|
||
Hiro, based on the last few comments, can you take a look at what's happening in current nightly that keeps us compositing when the downloads panel has been closed, and its content
slot has been marked display: none
? (I just checked and that change does now take effect - though the downloads panel itself seems to still be display: -moz-popup
, it's unclear to me if that's expected - Emilio, did you see that when working on bug 1714846?)
Comment 26•5 months ago
|
||
Yeah, that's kind of expected. In the past when mentioning this to Neil he mentioned that the current design of popups not being display: none
when closed was to avoid recreating native widgets over and over for e.g., the context menu. Maybe the trade-offs are different now?
![]() |
||
Updated•5 months ago
|
Assignee | ||
Comment 27•5 months ago
|
||
The animation in question here is on this downloadsPanel, right? As far as I can tell, it's the animation keeps running on the compositor and I can't see display:none style on the element after the popup was closed. Oh now I see what you meant, yeah the display style is: -moz-popup...
Assignee | ||
Comment 28•5 months ago
|
||
I am assuming what Gijs wanted to know is why the animation keeps running after closing the popup. The answer is;
- The animation is a CSS transition running on the compositor, we let CSS transitions keep running on the compositor until an explicit notification comes from the main-thread
- Though this is a guess, in the popup case the notification is tried to be sent after popup closed thus any display list building stuff won't happen thus the notification will never be arrived to the compositor
So, I guess setting transition:none just before closing the popup would solve this issue? (IIRC there's a such popup blah event)
Comment 29•5 months ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #27)
The animation in question here is on this downloadsPanel, right? As far as I can tell, it's the animation keeps running on the compositor and I can't see display:none style on the element after the popup was closed. Oh now I see what you meant, yeah the display style is: -moz-popup...
FWIW, this sounds very similar to bug 1739161, where the element is made zero size. In that testcase, the animation ends properly only when we take EndEmptyTransaction
painting path, which is only used with retained display lists.
Retained display lists are not used for popups, so this might be related.
Assignee | ||
Comment 30•5 months ago
|
||
(In reply to Miko Mynttinen [:miko] from comment #29)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #27)
The animation in question here is on this downloadsPanel, right? As far as I can tell, it's the animation keeps running on the compositor and I can't see display:none style on the element after the popup was closed. Oh now I see what you meant, yeah the display style is: -moz-popup...
FWIW, this sounds very similar to bug 1739161, where the element is made zero size. In that testcase, the animation ends properly only when we take
EndEmptyTransaction
painting path, which is only used with retained display lists.
Retained display lists are not used for popups, so this might be related.
That's a different, a problem about bug 1739161 is happening on the main-thread, it keeps restyling on the main-thread, on the other hand in this bug the animation isn't restyled on the main-thread.
Assignee | ||
Comment 31•5 months ago
|
||
I figured out a way to solve this. The way is to call ClearCachedWebrenderResources when the popup gets hidden. I was initially thinking that it's not a good way because of resource recreation cost what Emilio commented in comment 26, but we've already clear the resources when recreating the popup. So I am mostly 100% sure it's okay.
Updated•5 months ago
|
Comment 32•5 months ago
|
||
I'm a bit confused. I think this happens for every panel? Both based on the conversation here (that seems to suggest that this happens due to the transition
that is on the root of the panel/popup node, which isn't being display: none
'd), and because that's what this profile seems to show, AFAICT: https://share.firefox.dev/3sl7By8 , and because that's what the dupe (bug 1413763) says.
But the steps here make me think this is somehow DL-panel specific? Was that intentional when filing it?
Given this isn't DL-panel specific, I'm going to unblock bug 1733587 as it's unrelated, really? People in general are much more likely to open "any arrow panel" either before or after the changes on nightly right now.
AIUI, the outgoing opacity transition is intentional, so I don't think frontend and/or the toolkit css and custom element code can fix this, despite the attempt in bug 1714846, and we might need a fix like the one in attachment 9255857 [details] [diff] [review].
Reporter | ||
Comment 33•5 months ago
|
||
(In reply to :Gijs (he/him) from comment #32)
I'm a bit confused. I think this happens for every panel? Both based on the conversation here (that seems to suggest that this happens due to the
transition
that is on the root of the panel/popup node, which isn't beingdisplay: none
'd), and because that's what this profile seems to show, AFAICT: https://share.firefox.dev/3sl7By8 , and because that's what the dupe (bug 1413763) says.But the steps here make me think this is somehow DL-panel specific? Was that intentional when filing it?
It was intentional, but it was me being confused. I tried a bunch of other panels at the time and they didn't reproduce. Testing a bit more today, I can reproduce with the hamburger panel and the control center / identity panels. I can't reproduce with the profiler or pocket panels.
AIUI, the outgoing opacity transition is intentional, so I don't think frontend and/or the toolkit css and custom element code can fix this
I don't think front-end code can avoid the bug without removing the opacity & transform transitions entirely.
we might need a fix like the one in attachment 9255857 [details] [diff] [review].
Here is a profile with this attachment applied: https://share.firefox.dev/3pd4033 It confirms that this change does fix the bug.
We might be able to write a simple test for this using ChromeUtils.vsyncEnabled()
before/after the hamburger panel has been shown and hidden.
Comment 34•5 months ago
|
||
(In reply to Florian Quèze [:florian] from comment #33)
We might be able to write a simple test for this using
ChromeUtils.vsyncEnabled()
before/after the hamburger panel has been shown and hidden.
This seems to just always return true
for me on Nightly, on Windows, when run from the browser console, both when the panel is open and when it's not. How is it supposed to help here? And, is there an alternative (perhaps using profiler markers) ?
Reporter | ||
Comment 35•5 months ago
|
||
(In reply to :Gijs (he/him) from comment #34)
(In reply to Florian Quèze [:florian] from comment #33)
We might be able to write a simple test for this using
ChromeUtils.vsyncEnabled()
before/after the hamburger panel has been shown and hidden.This seems to just always return
true
for me on Nightly, on Windows, when run from the browser console, both when the panel is open and when it's not.
This always returns true because the browser console uses a CSS animation to make the cursor blink. When testing locally I used a setTimeout to workaround it. Eg. run setTimeout(() => console.log(ChromeUtils.vsyncEnabled()), 1000);
in the console and quickly move the focus away from the console to make the cursor disappear.
Comment 36•5 months ago
|
||
Comment 37•5 months ago
|
||
Setting a needinfo for Florian to come back to this when he's back from PTO, as I didn't really get very far. The patch I attached works, as far as I can tell, and the test passes but (a) it's kinda ugly and (b) the test also passes before the patch, so that's not much use. Despite Florian's extensive help, I didn't get very far trying to figure out why - I tried to use the profiler to find out what was happening in the automated test, but it doesn't seem to include profiles of the gpu process, even though when I check if the GPU process is running by using requestProcInfo
it is indeed running. If I try to collect shutdown profiles by failing the test, the browser crashes, without useful stacks. So I'm a bit lost, and I'm hoping Florian is in a better position to pick this up (but if anyone else has ideas, I'm sure they'd be appreciated too - we should really try to get this fixed sooner rather than later, considering the potential perf and power usage impacts).
Updated•5 months ago
|
Assignee | ||
Comment 38•5 months ago
|
||
I managed to make the test work (on Linux with xul.panel-animations.enabled=true). So what I've noticed is it seems the issue only happens when the popup is closed by a native mouse click event. And I am now unsure though, I've changed to use the download panel instead of the browser menu.
(Though I didn't want to assign this bug to me, but moz-phab forcibly did it)
Updated•5 months ago
|
Reporter | ||
Comment 39•4 months ago
|
||
(In reply to :Gijs (back Jan 4, 2022; he/him) from comment #37)
Setting a needinfo for Florian to come back to this when he's back from PTO, as I didn't really get very far. The patch I attached works, as far as I can tell, and the test passes but (a) it's kinda ugly and (b) the test also passes before the patch, so that's not much use.
Given that Hiro managed to make the test work, do you still need my help?
I tried yesterday Hiro's version of the test on my local Windows build. The test passes with the layout/xul/nsMenuPopupFrame.cpp
fix applied, and fails without. So it looks like it's testing what we expect.
I forced the test to fail to have a full profile of it including the GPU process: https://share.firefox.dev/3HvPUQA
One thing I noticed in this profile is that the initial waitForCondition lasted 3.6s, waiting for vsync to be disabled at the beginning of the test. This is due to vsync notifications being observed by a content process for an about:blank document (https://share.firefox.dev/3zl1Th3), I don't have a good understanding of what's causing that yet, but it's something to figure out in a separate bug.
Reporter | ||
Updated•4 months ago
|
Reporter | ||
Comment 40•4 months ago
|
||
(In reply to Florian Quèze [:florian] from comment #39)
One thing I noticed in this profile is that the initial waitForCondition lasted 3.6s, waiting for vsync to be disabled at the beginning of the test. This is due to vsync notifications being observed by a content process for an about:blank document (https://share.firefox.dev/3zl1Th3), I don't have a good understanding of what's causing that yet, but it's something to figure out in a separate bug.
I filed bug 1748466 for this.
Updated•4 months ago
|
Reporter | ||
Updated•4 months ago
|
Comment 42•4 months ago
|
||
Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/14f484747a66 Discard WebRender resources when popup hides. r=Gijs,emilio
Comment 43•4 months ago
|
||
Backed out changeset 14f484747a66 (Bug 1742797) for causing failures in nsMenuPopupFrame.cpp CLOSED TREE
Log: https://treeherder.mozilla.org/logviewer?job_id=363168402&repo=autoland&lineNumber=6130
https://treeherder.mozilla.org/logviewer?job_id=363169212&repo=autoland&lineNumber=2846
Backout: https://hg.mozilla.org/integration/autoland/rev/6cb156d6c627e91795567571e689d92e65b28e92
Assignee | ||
Comment 44•4 months ago
|
||
Do'h. mView
in nsMenuPopupFrame is a dangling pointer... I will figure out a way to avoid the situation.
Assignee | ||
Comment 45•4 months ago
|
||
Move if (auto* widget = GetWidget()) {}
block before mView
gets destroyed by nsIFrame::DestroyFrom.
Comment 46•4 months ago
|
||
Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e7b991987ae5 Discard WebRender resources when popup hides. r=Gijs,emilio
Comment 47•4 months ago
|
||
bugherder |
Comment 48•4 months ago
|
||
Backed out for causing Bug 1748808
Comment 49•4 months ago
|
||
Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/09808a64bedd
Comment 50•4 months ago
|
||
This is really bad for a range of reason. Bumping the priority. Graphics team might have some thoughts here.
Assignee | ||
Updated•4 months ago
|
Assignee | ||
Comment 51•4 months ago
•
|
||
I did investigate two test failures, bug 1748788 and bug 1748808. Now I am pretty sure bug 1748808 is unrelated to this bug. Bug 1748788 is the only one remaining issue to land this bug.
To performance team, I will not spend time for bug 1748788.
Assignee | ||
Comment 52•4 months ago
|
||
A way to avoid bug 1748788 I can think of is to add a new route to invoke ClearAnimationResources and use it in HidePopup, it should just work.
Comment 53•3 months ago
|
||
Hey Bas, do you know somebody who might be able to help with bug 1748788? There's a nice power / CPU usage win patch here that's being blocked by it.
Comment 54•3 months ago
|
||
Jeff would be the best person to figure out who might be able to look at that I think.
Comment 55•3 months ago
|
||
Hiro, is this ready to reland now that bug 1748788 is fixed?
Assignee | ||
Comment 56•3 months ago
|
||
Pushed a new try run to see if there's any suspicious failure or not. https://treeherder.mozilla.org/jobs?repo=try&revision=1a10622da604d539f42a7ae867d03ff8fb0e96de
That one is a try auto run, so I might have to trigger additional tasks there.
Assignee | ||
Comment 57•3 months ago
|
||
The test for this bug, browser_panel_vsync.js failed. Something has been changed since the last land?
Assignee | ||
Comment 58•3 months ago
|
||
It looks like the vsync tick on the compositor thread has been less often than before. Thus even if ChromeUtils.vsyncEnabled() returns false, DownloadsPanel.panel.state
is not yet closed
. With an additional awaiting for popuphidden
the test passes properly. I also confirmed with the additional await the test still properly fails without the proper fix in nsMenuPopupFrame.cpp.
Pushed a new try run; https://treeherder.mozilla.org/jobs?repo=try&revision=cc9d09aa4020c0206b98b7f1b80989e17d15ae6c (but it looks like it hasn't yet been queued)
Assignee | ||
Comment 59•3 months ago
•
|
||
Though bug 1748788 was backed out now, the test browser_panel_vsync.js is quite flaky, I did confirm that resources for compositor animations has been discarded but VsyncSource gets enabled after the discard.
Florian, are you aware of the case where VsyncSource gets re-enabled for some reason?
Updated•3 months ago
|
Reporter | ||
Comment 60•2 months ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #59)
Sorry for the late reply, I was on PTO most of last week.
Though bug 1748788 was backed out now, the test browser_panel_vsync.js is quite flaky, I did confirm that resources for compositor animations has been discarded but VsyncSource gets enabled after the discard.
Florian, are you aware of the case where VsyncSource gets re-enabled for some reason?
Flaky in which way? When running locally on my Windows machine, I saw cases where the test timeouts, with the downloads panel still visible and a leftover popuphidden event listener. I haven't been able to reproduce with the profiler running, but my guess is we need to wait for something before sending the click that will trigger the popup closing.
If you are talking about the Linux failures ("TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_panel_vsync.js | vsync should still be off") on the try run from comment 58, I would say the easiest way forward is to get the try server to upload profiles of the failures.
I did that using ./mach try fuzzy browser/base/content/test/performance/browser_panel_vsync.js --env MOZ_PROFILER_STARTUP=1
and here are the results: https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=b821db08fc7f9d9ef7acd4e8aa3191122731d685
Here's one of the profiles from that try run: https://share.firefox.dev/3t2wsGZ
It shows that TestUtils.waitForCondition(() => !ChromeUtils.vsyncEnabled())
("TestUtils - waitForCondition succeeded after 0 retries - undefined") finished before hiddenPromise
("BrowserTestUtils - waitForEvent: popuphidden"). So at the time we do the final ok(!ChromeUtils.vsyncEnabled(), "vsync should still be off");
we don't have a TestUtils.waitForCondition
, and it's normal for vsync to take about 200ms before being disabled after something has visibly changed.
This change https://hg.mozilla.org/try/rev/4899258047a1156aecc42278fd1ba65e190ceb9a seems to be enough to make the test green on try (https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=cccfb7dd33b48a1419826a889a18eff916906fec) minus a few leaks on debug builds that are likely due to running with the profiler.
By the way, it would be nice to provide a message for the second TestUtils.waitForCondition
call at https://hg.mozilla.org/try/rev/a3c46d3d95e0f389944c128485baed798b864bf7#l2.53
Reporter | ||
Comment 61•2 months ago
|
||
(In reply to Florian Quèze [:florian] from comment #60)
When running locally on my Windows machine, I saw cases where the test timeouts, with the downloads panel still visible and a leftover popuphidden event listener. I haven't been able to reproduce with the profiler running,
I haven't been able to get a profile of the failure locally, but TV jobs on try could! https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=cccfb7dd33b48a1419826a889a18eff916906fec&searchStr=win&selectedTaskRun=HOzdo7EMRQWJ--QAjzOgXw.0
This profile contains both a passing run of the test and a failing run that timeouts: https://share.firefox.dev/3MsmFRY
The sequences of DOM events are different in these 2 cases. The passing case has popuphiding/popuphidden events https://share.firefox.dev/3CygCqE, but no mousedown or click events. The failing case has mousedown, click (and event dblclick!) events: https://share.firefox.dev/3HNLWCN but no popuphiding/popuphidden events.
Comment 62•1 month ago
|
||
The CPU consumption issue seems to have gotten worse in 100.0b. Bug 1714846 did make 97+ seem more responsive when a large DL was in progress but it seems to have regressed or something.
Description
•