Re: Proposed changes for Chrome Extension Sockets (Pausing, Flow Control, STARTTLS)

[Chris Bentzel]

+apps-dev

On Thu, May 21, 2015 at 1:56 PM 'Lally Singh' via net-dev <net...@chromium.org> wrote:

We've found some API and performance issues when using chrome extension (actually app, I think) sockets in uProxy.  I've been working on trying to sort them out with minimal intrusion on the way things work now and the exposed APIs.  I've got some reasonable prototypes and wrote it up in

https://docs.google.com/a/google.com/document/d/1sAWQZ0Dvbz-xl_zo6Qv4CARIBA5BLrb0SKPiao8sy3c/edit?usp=sharing

We've got some nasty workarounds for the flow control issues we're currently facing (using setPaused as a backstop), but have gotten lucky in what we need out of TLS sockets.   It'd be nice to not have to do that -- our workarounds aren't performant or robust.

Comments invited, requested, and hoped for :-)

Cheers,

$ ls

Lally Singh |

Google Ideas |

Engineering |

la...@google.com

 

--
You received this message because you are subscribed to the Google Groups "net-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to net-dev+u...@chromium.org.
To post to this group, send email to net...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/net-dev/CAOvQDbLOktoQpYKVG6%2BQoPURm5x-MXLmG1Z2Ri6Ek0Dhvod7jw%40mail.gmail.com.

[Ryan Sleevi]

+several more explicit CC's, particularly people who have either been looking at the Streams API or about the general //net stack exposure of sockets in a performant way

Domenic, Misha, Matt, could you take a look at this? The set of problems being dealt with I believe are very similar to problems you're dealing with or have dealt with, and it would be great to get added perspective, insight, and review of this.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/net-dev/CAAuiYA8sji6dhJx3W3zojU-QziZFRTFMiyayg9cW_GG0St%3Dk4w%40mail.gmail.com.

[Mike Tsao]

+Renaud Paquay +Robert Ginda

[Lally Singh]

Hi, sorry for my delay.

We're already using setPause to apply backpressure, and it works horribly.  The problem is that you don't know how far behind you are processing data vs how much has come in.  We don't know how many other OnReceive events are already queued up for us at any time, so by the time we're pausing, we're either guessing that there's too much data coming in, or we're already mid-deluge.

On a separate note: Looking at Renaud's comments about an async read(n) similar to node.js, this makes more sense to me than what I had in the doc.  It'll avoid the entire Peek/Consume part of the API and give us something pretty reasonable, I think.  I'll write it up as an alternative in the doc.

Cheers,

$ ls

Lally Singh |

Google Ideas |

Engineering |

la...@google.com

 

On Fri, May 22, 2015 at 4:32 PM, Domenic Denicola <dom...@google.com> wrote:

On Fri, May 22, 2015 at 4:19 PM Domenic Denicola <dom...@google.com> wrote:

This is indeed very interesting. From the streams API perspective, it is designed to handle exactly these kind of problems. The design we landed on was implicit backpressure, in that you pull from the stream by calling a promise-returning read() method, and if you stop calling read() for a while, backpressure will be applied. (Although, the fetch response body stream shipping to the public web in M43 doesn't yet support backpressure.) The setConsumed API is also quite interesting in that we have been debating whether the streams API needs something like that; I will leave some related comments in the doc.

I am not sure how helpful streams can be here though unless you guys are up for reimplementing the TCP API on top of streams, which seems doubtful.

After reading/thinking a bit more, I may have been too hasty. Let me explain.

The streams API is designed to wrap up other APIs (called underlying sources) into a uniform ReadableStream interface. You could do a lot of work to reimplement the TCP API as a stream on top of a C++ underlying source, as I alluded to. Or, you could do much less work to create a wrapper ReadableStream that uses the chrome.sockets.tcp API as a JS underlying source.

If I am reading the doc correctly, the problem is not that there is no way for clients to apply backpressure---setPause() will do that. The problem is that expecting clients of chrome.sockets.tcp to keep track of how overhelmed they're getting, and call setPause() as appropriate, is not realistic.

In that case, wrapping up chrome.sockets.tcp into a JS ReadableStream would solve the problem exactly, as this is precisely the kind of thing we take care of in adapting from raw underlying source APIs to the ReadableStream API. For example, see https://streams.spec.whatwg.org/#example-rs-push-backpressure (which I may just go edit to use chrome.tcp.sockets since it's a nice real-world example).

WDYT?

 

Maybe some of the implementation work can play a part; Yutaka and Takeshi can likely speak to that better. Honestly, you guys probably have more to teach us, in terms of being real users with pain points in an existing nonstandard API.

Thanks for looping me in, Ryan.

[Lally Singh]

OK, I'm not understanding. The ReadableStream is in javascript?  How could it do a better job?

Cheers,
$ ls

On Jun 10, 2015 4:48 PM, "Domenic Denicola" <dom...@google.com> wrote:

On Wed, Jun 10, 2015 at 12:19 PM Lally Singh <la...@google.com> wrote:

Hi, sorry for my delay.

We're already using setPause to apply backpressure, and it works horribly.  The problem is that you don't know how far behind you are processing data vs how much has come in.  We don't know how many other OnReceive events are already queued up for us at any time, so by the time we're pausing, we're either guessing that there's too much data coming in, or we're already mid-deluge.

Right, that's exactly the problem that the ReadableStream wrapper would solve for you :).

[Lally Singh]

Hello Everyone.

Thank you for your feedback!  I've updated the doc to propose a read() API that's available only when the socket's paused.    Talking to Dominic about ReadableStreams, that's a really good option when it's available, but it's still useful (especially for our team, who's getting hurt by this now) to fix the current system now.

+Ryan: have you had a chance to have a look?

Cheers,

$ ls

Lally Singh |

Google Ideas |

Engineering |

la...@google.com

 

On Thu, Jun 11, 2015 at 5:19 PM, Domenic Denicola <dom...@google.com> wrote:

ReadableStream is provided by the browser, but its behavior is customizable. In particular, it keeps track of the chunks that are currently enqueued in the stream (i.e., how much data has come it) but not read from it by the consumer (i.e., how far behind you are processing data). If that exceeds a configurable threshold, it can perform an action (like setPause). Then when that falls below a threshold, it can un-pause.

In particular, see how you would create a ReadableStream wrapping the socket API at https://streams.spec.whatwg.org/#example-rs-push-backpressure. (The public API for readable stream is at https://streams.spec.whatwg.org/#rs-intro)