Skip to content

fix: 1 mili sec delay for read and write#128418

Open
Neo-vortex wants to merge 1 commit into
dotnet:mainfrom
Neo-vortex:fix-serial-read-delay
Open

fix: 1 mili sec delay for read and write#128418
Neo-vortex wants to merge 1 commit into
dotnet:mainfrom
Neo-vortex:fix-serial-read-delay

Conversation

@Neo-vortex

Copy link
Copy Markdown
Contributor

Fixes: #45635

Problem

When calling ReadByte() multiple times while data is already available in the
driver buffer, each call incurs a minimum 1ms delay. For example:

// 4 bytes already sitting in driver buffer — costs 4ms instead of ~0ms
port.ReadByte(); // 1ms
port.ReadByte(); // 1ms
port.ReadByte(); // 1ms
port.ReadByte(); // 1ms

The same issue affects writes. When writing chunks of data in sequence,
each Write() call enqueues a request and waits for the next poll cycle
to flush it, paying the same 1ms cost per write even when the driver's
send buffer has room and could accept the data immediately:

// driver send buffer has room — but still costs 1ms per write
port.Write(buf1, 0, buf1.Length); // 1ms
port.Write(buf2, 0, buf2.Length); // 1ms
port.Write(buf3, 0, buf3.Length); // 1ms

This affects any scenario where port.BytesToRead > 1 and the caller reads
in a loop using ReadByte() or any read smaller than the available data.
Reported on Linux and WSL2. Windows is not affected as it uses a different
code path.

Root cause

Two bugs working together in SerialStream.Unix.cs:

Bug 1 — IOLoop only processes one request per poll cycle.
After a successful PollEvents call confirms data is ready, DoIORequest is
called once, completes one request, and returns. The loop then goes back to the
top and calls PollEvents(timeout: 1ms) again for the next request — even
though data is still sitting in the buffer ready to go. Each byte pays the full
1ms poll cost independently. This applies to both reads and writes.

Bug 2 — ProcessRead and ProcessWrite return 0 for both "queue empty"
and "EWOULDBLOCK".

Even if we tried to drain multiple requests in one shot, there was no way for
DoIORequest to know whether 0 meant "nothing left to read/write right now"
vs "the queue is empty". This made it impossible to implement a correct drain
loop without risking spinning forever.

Fix

Four coupled changes, all required together:

ProcessRead — on EWOULDBLOCK, explicitly return -1 instead of falling
through to return 0 at the bottom. This gives the caller a distinct signal
that no data is available right now, separate from the queue-empty case.

ProcessWrite — same change on the write side. Return -1 on EWOULDBLOCK
instead of falling through.

DoIORequest — remove Debug.Assert(ret >= 0) since -1 is now a valid
and meaningful return value. Propagate -1 immediately to the caller when
received from the processor delegate.

IOLoop — replace the single DoIORequest call with a drain loop for both
reads and writes. After a single successful PollEvents, keep processing
requests until either the queue is empty (0) or the driver signals no more
data right now (-1). _totalBytesRead is updated inside the loop so it only
accumulates real positive reads.

// before — one request per poll cycle
if (events.HasFlag(Interop.PollEvents.POLLIN))
{
    int bytesRead = DoIORequest(_readQueue, _readQueueLock, _processReadDelegate);
    _totalBytesRead += bytesRead;
}

if (events.HasFlag(Interop.PollEvents.POLLOUT))
{
    DoIORequest(_writeQueue, _writeQueueLock, _processWriteDelegate);
}

// after — drain all ready requests in one shot for both reads and writes
if (events.HasFlag(Interop.PollEvents.POLLIN))
{
    while (!IsReadQueueEmpty())
    {
        int bytesRead = DoIORequest(_readQueue, _readQueueLock, _processReadDelegate);
        if (bytesRead <= 0) break; // 0 = queue empty, -1 = EWOULDBLOCK
        _totalBytesRead += bytesRead;
    }
}

if (events.HasFlag(Interop.PollEvents.POLLOUT))
{
    while (!IsWriteQueueEmpty())
    {
        if (DoIORequest(_writeQueue, _writeQueueLock, _processWriteDelegate) <= 0) break;
    }
}

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-io-ports
See info in area-owners.md if you want to be subscribed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.IO.Ports community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SerialPort read latency on linux

2 participants