flow control over websockets

[marvinthepa]

I have two items for discussion about https://xtermjs.org/docs/guides/flowcontrol/, more precisely the "Flow control over websockets" section

1. as the guide says "This means we cannot reliably implement a flow control for those buffers directly (the websocket protocol also doesn’t offer hooks for this)", would it make sense to mention that Chrome has WebSocketStream now, which offers the required functionality?
2. I had a bit of a hard time trying to figure out what is meant by "Here it is important that ackCondition on the client side and condition / stopCondition on the server side agree about the things to measure or the flow control will block the stream sooner or later." - There are no hints what ackCondition, condition, and stopCondition are supposed to be exactly. Could this be clarified in any way?

I tried to derive them from the examples in the other sections, and ended with an implementation similar to this:

  // client
  termWrite(chunk) {
    this.written += chunk.length;
    if (this.written <= BYTE_LIMIT) {
      return this.terminal.write(chunk);
    }

    let written = this.written;
    this.terminal.write(chunk, () => this.ack(written));
    this.written = 0;
  }

  ack(written) {
    this.ws.send(JSON.stringify({ ack: written }));
  }

  # server
  def ack(bytes)
    return unless bytes

    @written = [@written - bytes, 0].max

    unless stop_condition
      resume
    end
  end

  def stop_condition
    @written > BYTE_LIMIT * 1.5
  end

  def pty_read(output)
    @ws.send(JSON[{ :data => output }])
    @written += output.length

    if stop_condition
      pause
    end
  end

with BYTE_LIMIT the same on both sides. It seems to work, but I have no idea if "the client side and condition / stopCondition on the server side agree about the things to measure", or if "the flow control will block the stream sooner or later"..

I also tried to figure out a way it can be combined with the approach from the "Ideas for a better mechanism", but wasn't quite sure if this is even possible..

[jerch]

@marvinthepa Ah sorry, I admit that the websocket flow control part is a bit vaguely phrased, as it does not show a real implementation. Meant is the following:

• ackCondition: condition on client side, that evals to true, once the server would expect to get an ACK from the client (e.g. after n bytes)
• stopCondition: condition on server side, that evals to true, once the server has m pending bytes (pending in the sense of not being ack'ed yet by the client), here the server should stop PTY transport
• ACK chunk / isAck(chunk): special message denoting an ACK, which should resume PTY transport, if stopCondition isnt set anymore

The notice

...the client side and condition / stopCondition on the server side agree about the things to measure...

is just a friendly reminder, that an implementation must take care to measure the same entities on both ends, thus all messages accounted on one end also have to be accounted on the other side the same way, even with the same type (e.g. counting raw bytes on one side vs. counting string chars on the other side is a bad idea, encoding differences would stack up).

Your example looks good to me, and if it works for you, then you are all set. Theoretically sending the number of transmitted bytes in the ACK message is not needed, if you define a fixed BYTE_LIMIT value on both sides. Ofc sending it along can give a few advantages, e.g. if you have a faulty connection and want to implement more sophisticated integrity checks. (But thats hardly an issue anymore with HTML/websockets).

I also tried to figure out a way it can be combined with the approach from the "Ideas for a better mechanism", but wasn't quite sure if this is even possible.

The part "Ideas for a better mechanism" mostly shows ideas, how to overcome some perf issues from callback-based impls in nodejs, esp. around kernel ctx switches and general callback pressure (function call overhead). Idk, if thats relevant for you, since you use JSON as transport format, which should show a much worse perf impact here.

And last but not least - if you have a better / more comprehensible example, feel free to open a PR for it here https://github.com/jerch/xtermjs.org

---

Sidenote - if you need high throughput over websocket, use raw bytes instead, and do a little bit of prebuffering of PTY output before wrapping data into websocket packages.

[marvinthepa]

The part "Ideas for a better mechanism" mostly shows ideas, how to overcome some perf issues from callback-based impls in nodejs, esp. around kernel ctx switches and general callback pressure (function call overhead). Idk, if thats relevant for you, since you use JSON as transport format, which should show a much worse perf impact here.

Sidenote - if you need high throughput over websocket, use raw bytes instead, and do a little bit of prebuffering of PTY output before wrapping data into websocket packages.

Thanks for the hint, I'll give it a try.

I mostly used JSON because I replaced an old implementation based on shellinabox, which used JSON.
We also need authentication and, as the flow control guide says:

Furthermore the ACK message needs to be marked as a special message to not get confused as normal data (custom message protocol).

And I was too lazy to think about how to implement this in a binary protocol 😁. Maybe I will just implement some buffering.

[jerch]

Ah well, JSON should get you going nonetheless. But it s*x quite hard on throughput perf due to the ongoing forth and back serialization. I dont remember the exact numbers from my local tests, but it was something along >>100 MB/s for a raw byte protocol vs. up to 20 MB/s with JSON based messages.

Buffering can lower the impact of JSON serialization too - depending on how the running application produces its output, it can go down to a single byte per PTY read (worst case) creating a really bad call overhead (mostly from serialization and websocket packaging). With a small buffer merging those tiny data chunks into slightly bigger ones, you can take away lots of that additional nonsense work.

A binary protocol could be shaped really simple here - you basically have 3 message types to separate - data, ACK & resize. A tiny binary protocol could just prepend a single messagetype byte, on which you can select on receiver side etc.

Edit: Originally I planned to showcase a proper flowcontrol, buffering & binary protocol in the repo demo, but we soon figured, that this is beyond xterm.js, as it highly depends on the server side impl. Thats also the reason, why the websocket paragraph got only those pseudocode remarks in the docs.

[marvinthepa]

A binary protocol could be shaped really simple here

I went for another approach: I stayed with JSON in text frames for the control messages, and changed the data messages to binary frames.

However, My measurements actually lead to poorer throughput when compared to JSON, so I guess I have some other issue in my implementation (or in the way I am measuring). As the performance seems to be "good enough" anyway and I have other stuff to do, I will not investigate this further at this point.

Thanks for the insight anyway. I have had nothing but good experiences with this project, you rule ❤️ .