1356956 - Label the messages under PCompositorBridge

Status: RESOLVED WONTFIX

(Core :: Graphics, enhancement, P3)

Description

[Kevin Chen[:kechen] (UTC + 8)]

Since Bug 1333968 has added a new API to label messages individually, we can start working on labeling other messages in PCompositorBridge.

Comment 1

[Andrew Overholt [:overholt]]

This is showing up as part of the remaining 3% of unlabelled runnables we need to get to 99% labelled. We think the Quantum DOM concurrent scheduler will not be effective until we've got 99% of runnables labelled. Can the priority here please be increased?

Comment 2

[Peter Chang[:pchang]]

(In reply to Andrew Overholt [:overholt] from comment #1)
> This is showing up as part of the remaining 3% of unlabelled runnables we
> need to get to 99% labelled. We think the Quantum DOM concurrent scheduler
> will not be effective until we've got 99% of runnables labelled. Can the
> priority here please be increased?

Vincent is going to check what else we need to add under PCompositorBridge.

Comment 3

[Vincent Liu[:vliu]]

Here is a list to show all IPDL messages except didComposite from Parent to Child in CompositorBridge channel.

   InvalidateLayers()
   RemotePaintIsReady()
   UpdatePluginConfigurations()
   CaptureAllPlugins()
   HideAllPlugins()
   ParentAsyncMessages()
   ObserveLayerUpdate()
   SharedCompositorFrameMetrics()
   ReleaseSharedCompositorFrameMetrics()

   Just like description in the beginning, Bug 1333968 has added a new API to label DidComposite message. For others, we can follow the similar way to implement. For details, I still need to take some time to study.

Comment 4

[Wesly Huang (EPM)]

Sorry for late, offline discussed w/ Peter and Vincent is on it as above.

@Peter, just to make sure the timeline set for this is "before" 57 entering beta, so Sep. 19th, right?

@Andrew, if you have other expected date than Sep. 19th please let us know, thanks.

Comment 5

[Peter Chang[:pchang]]

(In reply to Wesly Huang (Firefox EPM) from comment #4)
> Sorry for late, offline discussed w/ Peter and Vincent is on it as above.
> 
> @Peter, just to make sure the timeline set for this is "before" 57 entering
> beta, so Sep. 19th, right?
Yes, that's the date what we discussed.
> 
> @Andrew, if you have other expected date than Sep. 19th please let us know,
> thanks.

Comment 6

[Vincent Liu[:vliu]]

(In reply to Andrew Overholt [:overholt] from comment #1)
> This is showing up as part of the remaining 3% of unlabelled runnables we
> need to get to 99% labelled. We think the Quantum DOM concurrent scheduler
> will not be effective until we've got 99% of runnables labelled. Can the
> priority here please be increased?

Hi Andrew,

1. Could you please offer the list about the remaining 3% of unlabelled runnables? It would be helpful working on labeling those messages.
2. There are some other gfx labeling bugs in [1] we still put in our backlog. Besides Bug 1356956, do you think others in [1] we also need to label it?

[1]: https://wiki.mozilla.org/TPEGFX/Bugs#GFX_labeling

Comment 7

[Andrew Overholt [:overholt]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1382172#c16 has some recent telemetry data on top unlabelled runnables. You could also check with Bevis Tseng who usually has an updated list. I'd just consult that for the prioritized list.

Peter, the earlier the better here. I was hoping for *August* 19th, not September 19th :)

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Andrew Overholt [:overholt] from comment #7)
> https://bugzilla.mozilla.org/show_bug.cgi?id=1382172#c16 has some recent
> telemetry data on top unlabelled runnables.
Here is the complete list I run last Friday. (8/4)
https://gist.github.com/bevis-tseng/2314117220caa2685bc50930cfb50142
PCompositorBridge::Msg_ParentAsyncMessages is the highest unlabeled message in PCompositorBridge protocol.

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

Per discussed offline with :vliu and :jerry, PCompositorBridge::Msg_ParentAsyncMessages only updates internal hashtable of the textures without touching anything in the content js, so SystemGroup can be used to label this message.
To specify an EventTarget for a specific message, we could override the IToplevelProtocol::GetSpecificMessageEventTarget() in ContentBridgeChild similar to what have done in ContentChild:
http://searchfox.org/mozilla-central/rev/4b79f3b23aebb4080ea85e94351fd4046116a957/dom/ipc/ContentChild.cpp#3553-3561

Comment 10

[Vincent Liu[:vliu]]

Attachment: 0001-Bug-1356956-Labeling-for-ParentAsyncMessages-in-PCom.patch

(In reply to Bevis Tseng [:bevis][:btseng] from comment #9)
> Per discussed offline with :vliu and :jerry,
> PCompositorBridge::Msg_ParentAsyncMessages only updates internal hashtable
> of the textures without touching anything in the content js, so SystemGroup
> can be used to label this message.
> To specify an EventTarget for a specific message, we could override the
> IToplevelProtocol::GetSpecificMessageEventTarget() in ContentBridgeChild
> similar to what have done in ContentChild:
> http://searchfox.org/mozilla-central/rev/
> 4b79f3b23aebb4080ea85e94351fd4046116a957/dom/ipc/ContentChild.cpp#3553-3561

As discussed, can you please have a review for the patch? Thanks

Comment 11

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8896121 [details] [diff] [review]
0001-Bug-1356956-Labeling-for-ParentAsyncMessages-in-PCom.patch

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

r=me after moving this patch to a new bug.

This patch only labels ParentAsyncMessages instead of all messages to be addressed in this bug.
Please file another bug with this patch and block this meta bug instead, thanks!

Comment 12

[Vincent Liu[:vliu]]

(In reply to Bevis Tseng [:bevis][:btseng] from comment #11)
> Comment on attachment 8896121 [details] [diff] [review]
> 0001-Bug-1356956-Labeling-for-ParentAsyncMessages-in-PCom.patch
> 
> Review of attachment 8896121 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me after moving this patch to a new bug.
> 
> This patch only labels ParentAsyncMessages instead of all messages to be
> addressed in this bug.
> Please file another bug with this patch and block this meta bug instead,
> thanks!

In case we have other messages under PCompositorBridge need to be labeled in the future, I will still keep this bug active. Based on this, bug 1389428 was filed for tracking ParentAsyncMessages labeling.

Comment 13

[Vincent Liu[:vliu]]

I will lower the priority of this bug because bug 1389426 was filed to add labeling for ParentAsyncMessages.

Comment 14

[Vincent Liu[:vliu]]

I will close this bug since we don't have any labeling relative bug on PCompositorBridge need to be fixed. Reopen it once there is any bug need to do.