Twice as many DidComposite messages as vsync messages in the content process

RESOLVED FIXED in Firefox -esr52

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: billm, Assigned: mattwoodrow)

Tracking

({regression})

unspecified
mozilla54
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox51 wontfix, firefox52 wontfix, firefox-esr5253+ fixed, firefox53 fixed, firefox54 fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(1 attachment)

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.
Priority: -- → P3
Whiteboard: gfx-noted
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

2 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+

Comment 3

2 years ago
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b75eb9cec112
Only send DidComposite to affected tabs. r=dvander

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b75eb9cec112
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Considering this regressed in 47 we should probably uplift.
Likely too late for Fx52 without strong motivation at this point (the RC1 build was already created on Monday).
Please request Aurora/ESR52 approval on this when you get a chance. Too late for Fx52 at this point.
Flags: needinfo?(matt.woodrow)
Assignee

Comment 8

2 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?
Attachment #8838997 - Flags: approval-mozilla-esr52?
Attachment #8838997 - Flags: approval-mozilla-beta?
Attachment #8838997 - Flags: approval-mozilla-aurora?
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+
Setting qe-verify- based on Matt's assessment on manual testing needs (see Comment 8).
Flags: qe-verify-
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+
You need to log in before you can comment on or make changes to this bug.