1958756 - Testcase inserting N @import stylesheets spends a lot of time on the parent-process.

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[Mayank Bansal]

Attachment: Import stylesheets2.HTML

N=10K
Firefox: https://share.firefox.dev/421qRlx (1.2s)
Chrome: https://share.firefox.dev/4cqS8Bw (600ms)

N=50K
Firefox: https://share.firefox.dev/3G98Yso (6s)
Chrome: https://share.firefox.dev/3R3ruov (3s)

N=100K
Firefox: https://share.firefox.dev/4jlSfk7 (15s)
Firefox Samply: https://share.firefox.dev/4loUuoI
Chrome: https://share.firefox.dev/4jlSjjR (6s)

We spend a large time on the parent-process. Overhead of taskcontrollers or something, or is it just errors ?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

So the parent process time seems to be due to data: channel notifying the parent process and so on. DataChannelParent::NotifyListeners.

Do you know what that is about Randell? I'd expect data: uris to do little to no work in the parent process given no networking should really be necessary? DataChannelParent seems like it was introduced to support redirect-to-data: uris but there's nothing like that going on here?

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

@valentin Do you know anything about DataChannelParent?

Comment 4

[Valentin Gosu [:valentin] (he/him)]

So DataChannelParent was only supposed to be used for redirects but in bug 1903060 we changed this to allow data channels to be displayed in devtools. This made the IPC connection be created for each DataChannelChild, which seems problematic indeed.
We might want to revert that and rethink the implementation.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1903060

:jdescottes, since you are the author of the regressor, bug 1903060, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 6

[Julian Descottes [:jdescottes]]

As said in https://bugzilla.mozilla.org/show_bug.cgi?id=1903060#c2, we can monitor the observer notification in content processes if needed. It will require some work to adapt DevTools and more so WebDriver BiDi. Or we make the IPC optional and only do it if the BrowsingContext is watchedByDevTools, or if a remote protocol is started (we don't really have a BC flag for BiDi)

I don't think this has anything to do with import + stylesheets, it's probably just related to the data URIs used in the test case.

We must have the same issue with File URLs, I think we use the same pattern.

Valentin, can you or someone from Necko set the priority here?

Comment 7

[Valentin Gosu [:valentin] (he/him)]

I think data URLs are used quite frequently in CSS, and we don't want to perf regress them.
file URLs are not as common on the web I presume, though we might want them to have the same behaviour.
I'll back out 1903060 in that case, and we can try monitoring the notification in the content process.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Julien, do you mind backing out the patches?
They there are some conflicts in the WPTs, otherwise I would have done it myself.
Thanks!

Comment 9

[Julian Descottes [:jdescottes]]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #7)

I think data URLs are used quite frequently in CSS, and we don't want to perf regress them.
file URLs are not as common on the web I presume, though we might want them to have the same behaviour.
I'll back out 1903060 in that case, and we can try monitoring the notification in the content process.

I don't think we can simply backout the patch, as it will break both DevTools and BiDi tests. We also need to update both implementations to start reading the notifications from the content process. So I can take the bug, but I will need time to update both...

Comment 10

[Julian Descottes [:jdescottes]]

Attachment: Bug 1958756 - Back out patches for exposing data channel notifications in parent

We should avoid doing unnecessary IPC when the data channel can fully be
handled in content processes. DevTools and WebDriver BiDi can monitor the corresponding
observer notifications in content processes.

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: Bug 1958756 - [bidi] Monitor data channels in windowglobal network module

Depends on D246571

Comment 12

[Julian Descottes [:jdescottes]]

Attachment: Bug 1958756 - [devtools] Monitor data channels in network-events-content actor

Depends on D246572

See initial patch in the stack. Sending parent process notifications for
all data channels had a non-negligeable performance impact.
The necko changes are backed out and DevTools should now monitor the observer
notification both in parent and content processes.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03903beed57c
Back out patches for exposing data channel notifications in parent r=valentin,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/58d591d8f43c
[bidi] Monitor data channels in windowglobal network module r=webdriver-reviewers,Sasha
https://hg.mozilla.org/integration/autoland/rev/95ff39756993
[devtools] Monitor data channels in network-events-content actor r=devtools-reviewers,nchevobbe

Comment 14

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/03903beed57c
https://hg.mozilla.org/mozilla-central/rev/58d591d8f43c
https://hg.mozilla.org/mozilla-central/rev/95ff39756993

Comment 15

[Julian Descottes [:jdescottes]]

This should now be available in Nightlies on all platforms. Not sure how the initial benchmark was run, but maybe you can give it another try now?

Comment 16

[Mayank Bansal]

Profile with latest Nightly (which i think should have the fix from this bug). N=10000
https://share.firefox.dev/42Ss6CT

Comment 17

[Julian Descottes [:jdescottes]]

Looks like it's still an old nightly, buildid is from yesterday 20250424205049
Maybe we should check tomorrow. Correct binary should be at https://archive.mozilla.org/pub/firefox/nightly/2025/04/2025-04-25-09-15-26-mozilla-central/

But I did record profiles on my machine and I can see the parent process is no longer busy.
However I'm not sure what you were timing exactly?

Comment 18

[Mayank Bansal]

Samply profile with latest Nightly: https://share.firefox.dev/42NDMH4 (12s)
Chrome: https://share.firefox.dev/42SHHT3 (7s)
The time in parent-process is all gone now. I will close this bug as fixed and open a new bug for the relative slowness.