Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can the output buffer be inspected to see if its clogging up? #69

Open
Docteh opened this issue Aug 2, 2019 · 5 comments
Open

Can the output buffer be inspected to see if its clogging up? #69

Docteh opened this issue Aug 2, 2019 · 5 comments

Comments

@Docteh
Copy link

Docteh commented Aug 2, 2019

I didn't notice anything when I was poking at the WritableStream object, but I think it would be useful to be able to know how much is in the buffer before writing out a request for a status.

@reconbot
Copy link
Contributor

reconbot commented Aug 2, 2019 via email

@reillyeon
Copy link
Collaborator

reillyeon commented Aug 2, 2019

I'm still thinking through what the actual specification text will look like but at a high level there are three buffers in both the read and write directions:

  1. The hardware buffer in the UART. This is usually small. When an OS-level write operation succeeds this is where the data is.
  2. The software buffer in the browser. This is controlled by the buffersize option. When the Promise returned by a call to write() resolves this is where the data is.
  3. The WritableStream queue. This is unbounded. When write() returns this is where the data is.

The buffers for read work similarly, but transfer data in the opposite direction.

What is left out of this a way to ensure that data has been written out of the hardware buffers. In the POSIX API this is tcdrain(3). I am planning on mapping this to the WritableStream's close() method. When the Promise it returns is resolved then all of the data previously written to the stream is not only in the software buffer but has been cleared out of the hardware buffer. Similarly the abort() method can be used to discard the data in the buffers. Since both of these methods leave the stream in a closed state the SerialPort will immediately replace it with a new WritableStream which starts off with completely empty buffers through the entire system.

None of this is implemented yet in Chromium and is tracked by issues 989653 and 989656.

I'm reluctant to add a method to measure the amount of queued data because it is inherently racy given that the buffers are constantly being drained by the UART. I'm curious whether given the design above you would still also need a method to query this.

Edited to add: Lots of buffering isn't great. I recommend applications avoid buffering lots of data in the WritableStream by always awaiting the Promise returned by write().

@reconbot
Copy link
Contributor

reconbot commented Aug 3, 2019

I apologize I got my repos confused and was referring to node serialport.

Node serialports drain waits for all stream level writes and does a tcdrain call. Which sounds quite similar to what you’re thinking.

@Docteh
Copy link
Author

Docteh commented Aug 3, 2019

Thanks, I'll look more closely at the promises. It is quite possible my issue would be solved by looking at timers, and promises a bit more closely.

Actually with the age of the codebase I'm looking at, it's quite possible that it's been solved, and the commands looking at the buffer never got removed...

@reillyeon
Copy link
Collaborator

Since my comment above the buffer flushing and purging behavior I mentioned above has been implemented. Calling close() on the WritableStream will wait until all buffered data has been written while abort() will discard any data that has not yet been written. In either case the writable attribute is populated with a new WritableStream instance that can be used to continue writing to the port.

As for determining the amount of data that is currently queued for transmission there is still work left to do. As specified the bufferSize option should set the high water mark for the WritableStream so that bufferSize - desiredSize is the amount of data currently in the internal queue. However as @domenic and I have discussed this doesn't play well with the fact that in Chromium we actually set the high water mark to 1 and queue bytes for transmission outside of the WritableStream implementation. This means that back pressure is correctly applied when the transmit queue is full but doesn't give callers a useful indication of how much queue space is available. Fixing this will require allowing a custom algorithm for queueing bytes and getting the desired size to fill the queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants