BlobRegistry Stream sends the IPC from the main thread of the renderer process.
So FetchEvent.respondWith() is blocked by the main thread.
We should use mojo::DataPipe instead.
I'm curious if the change this issue is describing would still make sense in a world where blobs were mojo-ifyied in such a way as to no longer requiring going through the main thread? I don't know how exactly blobs are involved with respondWith, but it seems unfortunate if we end up working around a short-coming in the blob system, rather than fixing that short-coming (And we are beginning to really look into how to mojo-ify the blob system).
Current problem is "respondWith(stream) is using Stream provided by BlobRegistry.
BlobRegistry is living on the main thread and respondWith on the service
worker's thread contains a thread hop implicitly.".
I'm not familiar with the blob mechanism, but if you can separate it out from the main thread, we'll no longer have the threading problem.
I've created a patch for this issue (https://crrev.com/2703343002), and I'm happy if you would added comments to it.
Actually doesn't seem like this has anything do with blobs at all, it's just that streams are for whatever reason piped through BlobRegistry? So not sure if the blob mojo-ification would necessarily help you.
But still the same question remains: if there is a performance issue with streams in workers, isn't the correct approach to change the way all streams are handled to use mojo datapipes (if that is possible), rather than changing how streams are dealt with in this specific case? I.e. getting rid of the streams related methods from BlobRegister, and mojo-ifying them in some way that avoids the threading issues?
But I don't really know anything about this code whatsoever, or why it was using streams in the first place. And I have no idea what the relative effort is of mojoifying streams to use datapipes, vs only changing the way service workers stream data to use datapipes. So I'm not saying that your current approach is wrong, it just seems potentially sub-optimal.
The mechanism of blink::Stream was introduced by tyoshino@ in 2013 when he was implementing (old) Streams API that is different one from the current spec.
And when I implemented .respondWith(stream) feature, I reused the mechanism of blink::Stream.
But the old Streams API was removed from the code base, and respondWith is the only user of blink::Stream.
So when we will use mojo::DataPipe for respondWith(stream), we can remove the current mechanism of blink::Stream.
Comment 1 by horo@chromium.org
, Feb 10 2017