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

Status: RESOLVED FIXED

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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?

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: only-send-did-composite-to-affected-tabs

We only need to send DidComposite to a tab if it was contained within the Layer tree that was just composited.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b75eb9cec112
Only send DidComposite to affected tabs. r=dvander

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b75eb9cec112

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Considering this regressed in 47 we should probably uplift.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Likely too late for Fx52 without strong motivation at this point (the RC1 build was already created on Monday).

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora/ESR52 approval on this when you get a chance. Too late for Fx52 at this point.

Comment 8

[Matt Woodrow (:mattwoodrow)]

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.

Comment 9

[Gerry Chang [:gchang]]

Comment on attachment 8838997 [details] [diff] [review]
only-send-did-composite-to-affected-tabs

Fix a regression to avoid wasting CPU cycles. Beta53+.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e2752c5516c1

Comment 11

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Matt's assessment on manual testing needs (see Comment 8).

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8838997 [details] [diff] [review]
only-send-did-composite-to-affected-tabs

avoid unnecessary ipc, esr52+

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/29a1ad09a6ec