New issue
Advanced search Search tips

Issue 882425 link

Starred by 2 users
Status: Duplicate
Merged: issue 854902
Owner:
wanderview@chromium.org
Closed: Sep 11
Cc:
falken@chromium.org
kinuko@chromium.org
shimazu@chromium.org
bashi@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

service worker could start reading body blob before messaging main thread

Project Member Reported by wanderview@chromium.org, Sep 10 
I'm splitting this off from  https://crbug.com/881141#c5 :

Another optimization we can make here is to create and start filling the response DataPipe immediately in the service worker thread:

https://chromium-review.googlesource.com/c/chromium/src/+/1214684

In the current code the service worker sends the GotResponse() mojo message back to the main thread.  The main thread then starts to load the blob.  If the main thread is busy with js then the start of the blob reading can be delayed for long periods.  By starting the read immediately we can at least fill the DataPipe if the main thread is janking.

Comment 1 by wanderview@chromium.org, Sep 10 

From   https://crbug.com/881141#c9 :

> re c#5: I'm a bit worried that reading the blob data in the worker thread potentially makes service worker thread busier. Instead of reading it in the service worker thread, ServiceWorkerSubresourceLoader (which dispatches a FetchEvent to the service worker thread on a background thread) might be able to call ReadAll on the background thread and send the datapipe back to the main thread.

I can investigate that.  One of my concerns was what would happen if the service worker thread was terminated before the ReadAll() fired its completion callback.

Beyond that, though, is there a specific part of the ReadAll() that is expensive on the initiating thread?  AIUI the browser process fills the DataPipe directly and then the main thread will read out of it.  Is creating the DataPipe itself expensive?

Comment 2 by wanderview@chromium.org, Sep 10

Blockedon: 858908

Comment 3 by wanderview@chromium.org, Sep 11

Blockedon: -858908

Comment 4 by wanderview@chromium.org, Sep 11 

I've been looking into the background thread Makoto-san mentioned in his comment and it does appear the blob ReadAll() is being initiated there.  So its not blocking on the main thread as I asserted in my original analysis.  This was fixed by Kinuko-san in  bug 854902 .

I'm close to just duping this to  bug 854902 , but I'm trying to understand if/why performance traces seem improved with my CL here first.

Comment 5 by wanderview@chromium.org, Sep 11

Blockedon: -881141
Mergedinto: 854902
Status: Duplicate (was: Assigned)
I believe the gains I was seeing were due to removing the side data reading that took place before initiating the body read.  I'm going to dupe this bug and open a new one to investigate performing the two reads in parallel.

Sorry again for my confusion.
►

Sign in to add a comment