Open Bug 704447 Opened 13 years ago Updated 2 years ago

WebSockets: large file-backed Blob sends should not block socket thread

Categories

(Core :: Networking: WebSockets, defect, P3)

defect

Tracking

()

People

(Reporter: jduell.mcbugs, Unassigned)

References

Details

(Whiteboard: [necko-backlog])

Bug 676439 is adding the binary API for both arraybuffers and blobs, but for large files it will be inefficient and will block the socket transport thread for longer than it ought to.   This bug is to implement proper nonblocking IO to read/write blobs as they come in/out.
OS: Linux → All
Hardware: x86_64 → All
A tricky part here is that we can only do non-blocking IO when we return a Blob. That means that we should only stream to disk if someone sets .binaryType="blob".

The trickier part is that we have to deal with the situation that someone might change .binaryType from "blob" to "arraybuffer" between when we start receiving data and when we fire the event.

In that case we need to postpone the event (and any later incoming message packets) until we've read all the data into memory.
Blocks: 711205
I talked to Jonas about this and he suggested that we avoid doing anything to make WebSockets blobs disk-backed at least until we have better infrastructure in place for controlling how disk space is used by websites. In particular, Jonas noted that it is less important to support huge WebSockets messages than huge XHR blobs because with WebSockets the server should have a lot more control over the messages, and it is probably the case that the server can just break the blob into smaller pieces as necessary.

Also, I think (and I think Jonas agreed) that we should support the ability to stream websockets messages off the network, which might further reduce the usefulness of having disk-backed blobs for WebSockets.

Personally, I think we should just avoid doing disk-backed blobs at all for WebSockets, because WebSockets doesn't really need them (AFAICT) for anything that the application couldn't do itself (especially with the File API and/or IndexedDB), and because WebSockets doesn't have anything like Cache-Control:no-store like XHR has, so we don't have enough information to decide whether it is safe to store the data on disk.

OTOH, Jason made a good point that the developer can just avoid using the Blob type if he wants "no-store" semantics.

Still, I agree with Jonas that this is not an urgent thing that we need to address soon.
> I talked to Jonas about this and he suggested that we avoid doing anything to
> make WebSockets blobs disk-backed at least until we have better infrastructure
> in place for controlling how disk space is used by websites.

OK, I'm fine with that.  So for now I'm going to limit this bug to a special case that still merits optimization: AFAICT Websockets can still be told to send() a file-backed Blob (such as a file JS gets from a user-selected file dialog) of arbitrary size.  At the moment, we read such Blobs into RAM in a blocking call on the socket transport thread.  That's better than doing on the main thread, but we can do better using nsIStreamTransportService.

Note that XHR/Http also block the socket thread during large file uploads, and have been for years: see bug 690633:  so while websocket's doing it too is suboptimal, I don't know that this needs to be a high priority fix: I'm inclined to resolve other websockets API correctness issues and bugs first.
Summary: Improve large blob support for WebSockets → WebSockets: large file-backed Blob sends should not block socket thread
Whiteboard: [necko-backlog]
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P1
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: P1 → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.