Closed
Bug 1337548
Opened 8 years ago
Closed 8 years ago
Twice as many DidComposite messages as vsync messages in the content process
Categories
(Core :: Graphics, defect, P3)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: billm, Assigned: mattwoodrow)
References
Details
(Keywords: regression, Whiteboard: gfx-noted)
Attachments
(1 file)
1.20 KB,
patch
|
dvander
:
review+
gchang
:
approval-mozilla-beta+
jcristau
:
approval-mozilla-esr52+
|
Details | Diff | Splinter Review |
Telemetry is showing that the content process receives DidComposite messages at twice the rate as vsync messages. Specifically, the #1 and #2 runnables in the content process are:
('PCompositorBridge::Msg_DidComposite', 36314),
('PVsync::Msg_Notify', 19854)
The numbers are in millions of runnables across two days of nightly usage.
Matt told me that this is an unexpected result, so I'm filing it as a bug. We should investigate why there are so many DidComposite notifications.
Updated•8 years ago
|
Priority: -- → P3
Whiteboard: gfx-noted
Reporter | ||
Comment 1•8 years ago
|
||
I figured out why this is. We send the DidComposite message over IPC here:
http://searchfox.org/mozilla-central/rev/59cd73fbfa14384a81a4e3eb17d3881b372702c4/gfx/layers/ipc/CompositorBridgeParent.cpp#1847
The problem is that we're sending one message per tab, even for tabs that aren't visible. Even if the messages for invisible tabs do nothing, that's a lot of messages for the IPC system to handle. Also, it looks like the messages for the invisible tabs are doing actual work.
Matt, is this the expected behavior?
Flags: needinfo?(matt.woodrow)
Assignee | ||
Comment 2•8 years ago
|
||
We only need to send DidComposite to a tab if it was contained within the Layer tree that was just composited.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Attachment #8838997 -
Flags: review?(dvander)
Attachment #8838997 -
Flags: review?(dvander) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b75eb9cec112
Only send DidComposite to affected tabs. r=dvander
Comment 4•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 5•8 years ago
|
||
Considering this regressed in 47 we should probably uplift.
Blocks: 1245765
status-firefox51:
--- → wontfix
status-firefox52:
--- → affected
status-firefox53:
--- → affected
Keywords: regression
Comment 6•8 years ago
|
||
Likely too late for Fx52 without strong motivation at this point (the RC1 build was already created on Monday).
Comment 7•8 years ago
|
||
Please request Aurora/ESR52 approval on this when you get a chance. Too late for Fx52 at this point.
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8838997 [details] [diff] [review]
only-send-did-composite-to-affected-tabs
Approval Request Comment
[Feature/Bug causing the regression]: Unknown, old regression.
[User impact if declined]: Processing more runnables than necessary, wasted CPU cycles.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just removes pointless runnables from being sent.
[String changes made/needed]: None.
Flags: needinfo?(matt.woodrow)
Attachment #8838997 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
Attachment #8838997 -
Flags: approval-mozilla-esr52?
Attachment #8838997 -
Flags: approval-mozilla-beta?
Attachment #8838997 -
Flags: approval-mozilla-aurora?
Comment 9•8 years ago
|
||
Comment on attachment 8838997 [details] [diff] [review]
only-send-did-composite-to-affected-tabs
Fix a regression to avoid wasting CPU cycles. Beta53+.
Attachment #8838997 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•8 years ago
|
||
bugherder uplift |
Comment 11•8 years ago
|
||
Setting qe-verify- based on Matt's assessment on manual testing needs (see Comment 8).
Flags: qe-verify-
Comment 12•8 years ago
|
||
Comment on attachment 8838997 [details] [diff] [review]
only-send-did-composite-to-affected-tabs
avoid unnecessary ipc, esr52+
Attachment #8838997 -
Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Comment 13•8 years ago
|
||
bugherder uplift |
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•