Closed
Bug 1258440
Opened 8 years ago
Closed 8 years ago
tpaint is slowed on Linux and Windows waiting on CompositorParent's mPluginUpdateResponsePending
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: mconley, Assigned: jimm)
References
Details
Attachments
(1 file)
Here's a profile from a Windows XP machine running tpaint: http://people.mozilla.org/~bgirard/cleopatra/?zippedProfile=http://mozilla-releng-blobs.s3.amazonaws.com/blobs/Try/sha512/d32a78e3a0a6ad1a0431c144403151d899a4cd48eaf0c55063171a8342c8d965ed217de6c3f8f753ae99b1c1923ecfa15af953d89ed8f4b6bd0d9386d7e58c14#report=920482dc8fa61592a64fe98c9f12047fdd183808&filter=%5B%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A2691,%22end%22%3A3965%7D,%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A3215,%22end%22%3A3420%7D%5D&selection=0,7776,6,7,7777,7778,4,5,6,7,7777,8,9,10,11,12,13,14,15,16,17,7875,7876,7877,7878,7879,56,57,58,59,60,61,109,110,111,112,113,114,115,116,117,1392,1393,1394,1395,1396,1397,1398,1399,1400,1401,1402,7899,1695,5985,3609,3610,3611,6799 Notice the paint that occurs in the content process around the 3282ms mark. Now notice the composite that occurs shortly after at around the 3295ms mark. Then there's this big-ass pause while the content process polls the event loop, and then finally a DidComposite message comes down, and tpaint ends. That DidComposite message seems to be coming from a later composite - probably from the one that occurs around the 3325ms mark. So the question is: How come the DidComposite message is not sent / received closer to the 3295ms composite mark?
Reporter | ||
Comment 1•8 years ago
|
||
Okay, looking at when the MozAfterPaint fires in the parent, I think what's going on here is that the content is ready to paint earlier than the parent is, so GeckoMain is waiting for the parent to have painted before it can composite the two together (which makes sense). So I think this is RESOLVED INVALID.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 2•8 years ago
|
||
I'm wrong on this. Looking at the profiles, delayedStartup is being run before the big pause, so the parent _must_ have painted the window. So there's still a mystery here.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Reporter | ||
Comment 3•8 years ago
|
||
In order to fire the MozAfterPaint event, we need to paint, and we need to composite. The gap that I describe in comment 0 is because the compositor thread is waiting for mPluginUpdateResponsePending to equal to false before it starts compositing again. AutoResolveRefLayers is returning true for the pluginsUpdatedFlag here: https://dxr.mozilla.org/mozilla-central/rev/ea6298e1b4f7e22ce2311b2b6a918822f0adb112/gfx/layers/ipc/CompositorParent.cpp#1247 This is because we're entering this conditional: https://dxr.mozilla.org/mozilla-central/rev/ea6298e1b4f7e22ce2311b2b6a918822f0adb112/gfx/layers/ipc/CompositorParent.cpp#2472 And then we return true at the bottom of the UpdatePluginWindowState method, which causes pluginsUpdatedFlag to be set to true, which causes mPluginUpdateResponsePending to set to true, which causes us to bail out early from compositing. Note that the window that tpaint opens has no plugins loaded. In fact, no plugins should have started at all during any part of this test. https://dxr.mozilla.org/mozilla-central/rev/ea6298e1b4f7e22ce2311b2b6a918822f0adb112/gfx/layers/ipc/CompositorParent.cpp#2472 looks suspicious. It seems to assume that we had plugins displayed in the past, which is not true in the case of the tpaint window. Something is clearly wrong here, jimm. Got any idea what the right move is?
Flags: needinfo?(jmathies)
Summary: MozAfterPaint seems to not be fired soon enough after compositing layers from the parent and content → tpaint is slowed on Linux and Windows waiting on CompositorParent's mPluginUpdateResponsePending
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jmathies
Flags: needinfo?(jmathies)
Assignee | ||
Comment 4•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/41831/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/41831/
Attachment #8733597 -
Flags: review?(mconley)
Assignee | ||
Comment 5•8 years ago
|
||
Pretty simple fix that avoids the attempt to hide plugins that shouldn't be present. I've confirmed this doesn't regress hiding plugin windows that do exist in tabs the user switches away from.
Reporter | ||
Comment 6•8 years ago
|
||
Comment on attachment 8733597 [details] MozReview Request: Bug 1258440 - Don't attempt to hide plugin windows when switching trees if the previous remote layer tree didn't contain plugin windows. Fixes a tpaint regression. r?mconley https://reviewboard.mozilla.org/r/41831/#review38297 This looks like the right move to me, though I'm really not a gfx/ reviewer. If you're comfortable with me reviewing this though (and I think I've got a handle on this), r=me.
Attachment #8733597 -
Flags: review?(mconley) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/acd6d1120ea1
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 9•8 years ago
|
||
Looks like this helped tabpaint and tpaint. Nice find!
You need to log in
before you can comment on or make changes to this bug.
Description
•