Label the messages under PCompositorBridge

RESOLVED WONTFIX

Status

()

Core
Graphics
P3
normal
RESOLVED WONTFIX
7 months ago
28 days ago

People

(Reporter: kechen, Assigned: vliu)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [QDL][BACKLOG][GFX])

Attachments

(1 attachment)

(Reporter)

Description

7 months ago
Since Bug 1333968 has added a new API to label messages individually, we can start working on labeling other messages in PCompositorBridge.
Whiteboard: [QDL][BACKLOG][GFX]
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?
Flags: needinfo?(wehuang)
Flags: needinfo?(howareyou322)

Comment 2

4 months ago
(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.
Flags: needinfo?(howareyou322)
(Assignee)

Comment 3

3 months ago
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.

Updated

3 months ago
Priority: -- → P1
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.
Flags: needinfo?(wehuang)
Flags: needinfo?(overholt)
Flags: needinfo?(howareyou322)

Comment 5

3 months ago
(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.
Flags: needinfo?(howareyou322)
(Assignee)

Comment 6

3 months ago
(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
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 :)
Flags: needinfo?(overholt)
(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.
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
(Assignee)

Comment 10

3 months ago
Created attachment 8896121 [details] [diff] [review]
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
Attachment #8896121 - Flags: review?(btseng)
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!
Attachment #8896121 - Flags: review?(btseng) → review+
(Assignee)

Updated

3 months ago
Depends on: 1389428
(Assignee)

Comment 12

3 months ago
(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.
(Assignee)

Comment 13

3 months ago
I will lower the priority of this bug because bug 1389426 was filed to add labeling for ParentAsyncMessages.
Priority: P1 → P3
(Assignee)

Comment 14

28 days ago
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.
Status: NEW → RESOLVED
Last Resolved: 28 days ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.