1350660 - De-syncify layer id mapping in the compositor

Status: RESOLVED FIXED

(Core :: Graphics: Layers, enhancement)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

There's two pieces of low-hanging fruit for sync messages in the UI process. The first is the initial layer id mapping, which can be async as long as EnsureGPUReady() will ultimately block on it being received.

The second is that the allocation message is sent immediately before sending NotifyChildCreated, which is also sync. There's no need for that and we can create a combined sync message.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 1, combine messages

When using the GPU process, send a single message that performs both the layer tree mapping and the NotifyChildCreated logic.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 2, fold into init message

As for the initial mapping on startup, I think we can just fold this into the Init message, which is already async.

Comment 3

[Ryan Hunt [:rhunt]]

Comment on attachment 8851382 [details] [diff] [review]
part 1, combine messages

Review of attachment 8851382 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/ipc/RenderFrameParent.cpp
@@ +128,3 @@
>    } else if (XRE_IsContentProcess()) {
>      ContentChild::GetSingleton()->SendAllocateLayerTreeId(browser->Manager()->ChildID(), browser->GetTabId(), &mLayersId);
>      CompositorBridgeChild::Get()->SendNotifyChildCreated(mLayersId);

I'm not sure what the state of nested browser processes is, but if this code is converted we should be able to get rid of the NotifyChild message. Not sure if it's worth the work.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8851382 [details] [diff] [review]
part 1, combine messages

Bill, IPC peer r? for new sync message

Comment 5

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

Comment on attachment 8851382 [details] [diff] [review]
part 1, combine messages

Review of attachment 8851382 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/ipc/PCompositorBridge.ipdl
@@ +175,5 @@
>    // See bug 1316632 comment #33 for why this has to be sync. Otherwise,
>    // there are ordering issues with SendPLayerTransactionConstructor.
>    sync NotifyChildCreated(uint64_t id);
>  
> +  // This version of NotifyChildCreated also performs a layer tree mapping.

Can you copy the comment about "See bug 1316632 comment #33" to this message as well. I don't want to lose it if we drop NotifyChildCreated.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f033452487fc
When using the GPU process, combine layer ownership and window mapping into a single IPC message. (bug 1350660 part 1, r=rhunt, r=billm)
https://hg.mozilla.org/integration/mozilla-inbound/rev/13d070200dfe
Don't synchronously send the initial layer tree mapping list to the GPU process. (bug 1350660 part 2, r=rhunt)

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back these out because they were conflicting with bug 1351777 while I was trying to do merges this evening. Feel free to rebase and reland.

Comment 8

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bb665191fd2
Backed out 2 changesets for causing merge conflicts a=backout

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/7b2da53804c6
Backed out 2 changesets for causing merge conflicts a=backout

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/599e7c393321
When using the GPU process, combine layer ownership and window mapping into a single IPC message. (bug 1350660 part 1, r=rhunt, r=billm)
https://hg.mozilla.org/integration/mozilla-inbound/rev/8605e9e4f7bb
Don't synchronously send the initial layer tree mapping list to the GPU process. (bug 1350660 part 2, r=rhunt)

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/599e7c393321
https://hg.mozilla.org/mozilla-central/rev/8605e9e4f7bb