Closed Bug 1397181 Opened 7 years ago Closed 7 years ago

Label child actors allocation using SystemGroup

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

enhancement

Tracking

()

RESOLVED WONTFIX

People

(Reporter: bevis, Assigned: bevis)

References

Details

Attachments

(2 files, 1 obsolete file)

I'd like to restrict that all the allocation of child actors wouldn't notify any web content. The benefit is that we could label all the constructor ipc message using SystemGroup. If not, we should move this notification to a new runnable instead. Note: all the actors and its message can be labeled with other SechedulerGroup by overriding its IToplevelProtocol::GetConstructedEventTarget.
After further study of each AllXxxChild implementation, I didn't see any exception which touches the web content during actor child allocation. Hence, I'd like to have all the child actor allocation on main thread labeled with SystemGroup by default if not specified in IToplevelProtocol::GetConstructedEventTarget(). For future implementation which would like to touch the web content right after allocation, I'd suggest to separate this logic from allocation by defining a dedicated message/runnable to handle it instead. Treeherder result looks fine as well, btw: https://treeherder.mozilla.org/#/jobs?repo=try&revision=45f251b85f8ec1df013426916bacf0034201550b
Attachment #8904953 - Attachment is obsolete: true
Attachment #8905401 - Flags: review?(wmccloskey)
This is to fix the mistake I've made in Bug 1394350 which is a dead code to a constructor message according to the logic in IToplevelProtocol::GetMessageEventTarget(). Sorry for not thinking over it carefully. :\
Attachment #8905404 - Flags: review?(wmccloskey)
(In reply to Bevis Tseng[:bevis][:btseng] from comment #2) > Created attachment 8905401 [details] [diff] [review] > (v1) Patch: Part 1: Label child actors allocation using SystemGroup. > > After further study of each AllXxxChild implementation, I didn't see any > exception which touches the web content during actor child allocation. > Hence, I'd like to have all the child actor allocation on main thread > labeled with SystemGroup by default if not specified in > IToplevelProtocol::GetConstructedEventTarget(). > > For future implementation which would like to touch the web content right > after allocation, I'd suggest to separate this logic from allocation by > defining a dedicated message/runnable to handle it instead. > > Treeherder result looks fine as well, btw: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=45f251b85f8ec1df013426916bacf0034201550b After reading bug 1398420, I think I didn't think over carefully whether a system group runnable has a dependency to another labeled runnable even though this system group runnable didn't touch any web content. Besides, most unlabeled child actors are allocated proactively by content process instead of receiving a request from parent. The improvement of the labeling by this patch is mostly for PContent::Msg_PIPCBlobInputStreamConstructor__ID according to the telemetry analysis. Maybe we should put this patch on hold and label each constructor message individually after clarifying its dependency on other runnables.
Attachment #8905401 - Flags: review?(wmccloskey) → review-
Attachment #8905404 - Flags: review?(wmccloskey)
Priority: -- → P2
Set to WONTFIX per comment 4.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: