1567222 - [network markers] Network loads that went to a service worker never get a "stop" marker in the content process

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect, P2)

Description

[Markus Stange [:mstange]]

Profile: https://perfht.ml/2O2Ck0o

Steps to reproduce:

1. Make sure you have an up-to-date service worker for https://profiler.firefox.com/ installed by going to that address, waiting a bit, and then clicking the reload button if it appears.
2. Close all profiler.firefox.com tabs.
3. Start the profiler.
4. Open a tab and go to profiler.firefox.com .
5. Wait for it to load.
6. Capture a profile.

Expected results:
The network track in the content process should correctly show that the network was inactive once the page has loaded. And the panel should have network load bars that end at the right time.

Actual results:
https://perfht.ml/2O2Ck0o
Many network loading bars in the content process are extended to the very end of the visible range, implying that the network loads hadn't completed at the time the profile was captured.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Is this because some network markers may be generated on threads that are not being profiled?

Comment 2

[Julien Wajsberg [:julienw]]

Thanks for the bug Markus! Indeed this is something I sensed in bug 1541360 without really reproducing. Also I was seeing the problem mostly on parent processes, but in your example profile we clearly see the problem happens also on child processes.

Comment 3

[Julien Wajsberg [:julienw]]

NI myself to check if this is still happening.

Comment 4

[Julien Wajsberg [:julienw]]

Yes, this still does happen. I captured the profile [1] using the exact STR above using latest Nightly.
Some interesting bits: the parent process and the child process do not show the same data. This probably exhibits 2 different bugs.

Parent process:

• all requests to profiler.firefox.com probably got their "start" marker but not the end.
• requests to www.google-analytics.com also exhibit a weird behavior where they're all duplicated, but one of the duplication only has the "start" marker. Do they have a service worker too?

Content process:

• all requests to profiler.firefox.com got their "end" marker but not their "start" marker.
• requests to www.google-analytics.com also show a weird behavior where they're all redirected... to the same URL.

The devtools' network monitor seem to show the right data:

• requests to profiler.firefox.com go through the service worker.
• requests to www.google-analytics.com look normal.

[1] https://perfht.ml/3bcmPdt

Hey Andrew, I've been told you could maybe help us for the issues that are part of dealing with a Service Worker. It would be good enough to have some information about how the flow is different in necko when the requests are served by a SW, so that maybe we can do the fix ourselves.

Also it may be a good idea to wait until the rewriting is done, but I don't know about the timeframe so we may want to do it before too, so some information about that would be very useful :-)

Thanks!

Comment 5

[Julien Wajsberg [:julienw]]

We discussed about this with Andrew, so I'm removing this needinfo. Also I'm working on this these days.

Comment 6

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - Add a LOAD_STOP marker when a request goes through a service worker r=asuth!

Comment 7

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - Improve profiler tests for the service worker case r=canaltinova!

Depends on D107714

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/91df7c6b51d4
Add a LOAD_STOP marker when a request goes through a service worker r=asuth,necko-reviewers,valentin,gerald
https://hg.mozilla.org/integration/autoland/rev/777690075644
Improve profiler tests for the service worker case r=canaltinova

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/91df7c6b51d4
https://hg.mozilla.org/mozilla-central/rev/777690075644

Comment 10

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] Improve the profiler's serviceworker tool to make it easier to install different serviceworker implementations r=canaltinova!

Comment 11

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] Small changes to the caching serviceworker profiler marker test r=canaltinova!

Depends on D112206

Comment 12

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] Report service worker install errors, so that tests don't timeout r=canaltinova!

Depends on D112207

Comment 13

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] Improve the reporting of objectContainsOnly in case of failures r=canaltinova!

Depends on D112208

Comment 14

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] In tests, make the network markers filter more generic r=canaltinova!

Depends on D112209

Comment 15

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] Change the test utility to find the service worker thread, using a profiler marker r=canaltinova!

Depends on D112210

Comment 16

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler, network] Change how we pass a few properties to the network markers r=valentin!

This is simplifying how we pass the URI, and this is abstracting how we
pass the size.
In general the goal is to use properties and methods from
HttpBaseChannel, so that maybe in the future we could abstract marker
handling to that parent class.

Depends on D112211

Comment 17

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] Add a test with a serviceworker that has no fetch handlers r=canaltinova!

Depends on D112212

Comment 18

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] Add redirect markers when respondWith isn't called in the service worker's fetch handler r=canaltinova!,asuth!

Depends on D112214

Comment 19

[Julien Wajsberg [:julienw]]

Attachment: Bug 1567222 - [profiler] Add a test to check network markers for synthetized responses from a service worker r=canaltinova!

Depends on D112216

Comment 20

[Julien Wajsberg [:julienw]]

Here is an "auto try": https://treeherder.mozilla.org/#/jobs?repo=try&revision=05941a233f72170405e39fd5fb879bc8fc97ce4a
Let's see if this works :-)

Comment 21

[Julien Wajsberg [:julienw]]

New try with some fixes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=727c8f1aec7a25c2250cbb03b7301a6063fa0c8f

Comment 22

[Julien Wajsberg [:julienw]]

New try again, I missed one case: https://treeherder.mozilla.org/#/jobs?repo=try&revision=004e47396733da854d13266b6a7cb0bbb6811b57

Comment 23

[Julien Wajsberg [:julienw]]

New try: https://treeherder.mozilla.org/jobs?repo=try&revision=d6fa64daedc523b457e4a8d2f01d6f6e2ffe6540

This time, there's no intermittent left. The issue with tsan is weird given that all tests are skipped.

Comment 24

[Julien Wajsberg [:julienw]]

last try after rebase and beforel anding: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c2e0ba3bf7894cdef4e3582534fb1e11301cab0

If everything's fine I'll land the sequence of patches!

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9259b30793f8
[profiler] Improve the profiler's serviceworker tool to make it easier to install different serviceworker implementations r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/38e0508d04a1
[profiler] Small changes to the caching serviceworker profiler marker test r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/9db79f22f4a2
[profiler] Report service worker install errors, so that tests don't timeout r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/dde8a7d1b450
[profiler] Improve the reporting of objectContainsOnly in case of failures r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/85e994430343
[profiler] In tests, make the network markers filter more generic r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/33485f1d940b
[profiler] Change the test utility to find the service worker thread, using a profiler marker r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/6cb63cc7c95d
[profiler, network] Change how we pass a few properties to the network markers r=valentin,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/21b8a519213b
[profiler] Add a test with a serviceworker that has no fetch handlers r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/caf32cf2a12d
[profiler] Add redirect markers when respondWith isn't called in the service worker's fetch handler r=canaltinova,asuth,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/2ba472ddb892
[profiler] Add a test to check network markers for synthetized responses from a service worker r=canaltinova

Comment 26

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9259b30793f8
https://hg.mozilla.org/mozilla-central/rev/38e0508d04a1
https://hg.mozilla.org/mozilla-central/rev/9db79f22f4a2
https://hg.mozilla.org/mozilla-central/rev/dde8a7d1b450
https://hg.mozilla.org/mozilla-central/rev/85e994430343
https://hg.mozilla.org/mozilla-central/rev/33485f1d940b
https://hg.mozilla.org/mozilla-central/rev/6cb63cc7c95d
https://hg.mozilla.org/mozilla-central/rev/21b8a519213b
https://hg.mozilla.org/mozilla-central/rev/caf32cf2a12d
https://hg.mozilla.org/mozilla-central/rev/2ba472ddb892