1793940 - Performance overhead for waiting socket thread when requests fall back from SerivceWorker

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, task, P2)

Description

[Eden Chuang[:edenchuang]]

Attachment: Request fallback from ServiceWorker.png

Performance overhead when the fallback request from ServiceWorker.
Here we saw "waiting for socket thread time" around 4 ~ 1xx ms when fallback request from SW. However, we did not see this overhead when the request directly went to the network.

Comment 1

[Eden Chuang[:edenchuang]]

The overhead is most caused by redirection to the new channel.

When the ServiceWorker fallback the request, InterceptedHttpChannel creates a new nsHttpChannel for the request, then redirects to the new nsHttpChannel.
However, this channel redirection would cause HttpChannelParent/HttpChannelChild replacement. It means the redirection will go from Parent to Child to Parent to construct a new pair of HttpChannelChild/Parent and HttpBackgroundChannelChild/Parent, and it takes at least 3X ms. On a busy content process, it could take more time.

Fortunately, SW fallback doesn't modify the request and nsILoadInfo too much, only clears the nsILoadInfo's controller attribute. That means we probably have a chance to reuse the original nsHttpChannel that the channel before redirecting to InterceptedHttpChannel. When the SW fallback happens, just redirect to the original nsHttpChannel in the parent process only to avoid HttpChannelChild/Parent replacement.

Comment 2

[Eden Chuang[:edenchuang]]

Attachment: Bug 1793940 - Stop channel redirection propagation to child for ServiceWorker fallback requests. r=#necko-reviewers

Comment 3

[Eden Chuang[:edenchuang]]

Attachment: Bug 1793940 - Disconnect StreamFilters on HttpChannelChild before relink ServiceWorker fallback channel. r=#necko-reviewers

Depends on D159582

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99538a702473
Stop channel redirection propagation to child for ServiceWorker fallback requests. r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/9e62be2aef43
Disconnect StreamFilters on HttpChannelChild before relink ServiceWorker fallback channel. r=necko-reviewers,valentin

Comment 5

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/99538a702473
https://hg.mozilla.org/mozilla-central/rev/9e62be2aef43

Comment 6

[Eden Chuang[:edenchuang]]

Attachment: Performance improvment snapshot

Attach a snapshot of profilings to show the performance improvement.

We can probably have a telemetry probe to estimate the improvement by comparing the average normal redirection time and the average ServiceWorker fallback redirection time.

Comment 7

[Alex Finder]

== Change summary for alert #35854 (as of Sun, 30 Oct 2022 04:01:20 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
30%	reddit-billgates-ama.members LastVisualChange	linux1804-64-shippable-qr	cold fission webrender	1,376.67 -> 963.33
17%	reddit-billgates-ama.members FirstVisualChange	linux1804-64-shippable-qr	cold fission webrender	145.38 -> 120.00
12%	reddit-billgates-ama.members PerceptualSpeedIndex	linux1804-64-shippable-qr	cold fission webrender	420.77 -> 368.25
10%	reddit-billgates-ama.members SpeedIndex	linux1804-64-shippable-qr	cold fission webrender	304.44 -> 273.96

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=35854

Comment 8

[Eden Chuang[:edenchuang]]

Comment on attachment 9300101 [details]
Bug 1793940 - Disconnect StreamFilters on HttpChannelChild before relink ServiceWorker fallback channel. r=#necko-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Users would not have any impact if declined.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patches are not risky since they do not involve other code too much. It just gives a shortcut for ServiceWorker fallback redirection to improve the performance.
• String changes made/needed: No
• Is Android affected?: Yes

Comment 9

[Donal Meehan [:dmeehan]]

Comment on attachment 9298988 [details]
Bug 1793940 - Stop channel redirection propagation to child for ServiceWorker fallback requests. r=#necko-reviewers

Approved for 107.0b9

Comment 10

[Donal Meehan [:dmeehan]]

Comment on attachment 9300101 [details]
Bug 1793940 - Disconnect StreamFilters on HttpChannelChild before relink ServiceWorker fallback channel. r=#necko-reviewers

Approved for 107.0b9

Comment 11

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f1d24c65f337
https://hg.mozilla.org/releases/mozilla-beta/rev/21720300a22a