Skip to content

Commit

Permalink
Improve TCP backpressure handling on Windows (#4252)
Browse files Browse the repository at this point in the history
Our prior setting of backpressure for TCP writes on Windows was naive. It was
based purely on the number of buffers currently outstanding on an IOCP socket.
The amount of data didn't matter at all. Whether more data could be accepted or
not, also wasn't taken into consideration. This commit greatly improves the
situation by only applying backpressure when Windows tells us that it is
applying backpressure.

No more guessing.

Two runtime API methods have been updated on Windows.

The Windows version of `pony_os_writev` will now return the number of buffers
accepted or zero if backpressure is encountered. All other errors still cause
an error that must be handled on the Pony side of the API via a `try` block.

The Windows version of `pony_os_send` will now return the number of bytes
accepted or zero if backpressure is encountered. All other errors still cause
an error that must be handled on the Pony side of the API via a `try` block.

I consider these changes  non-breaking as previously, the return values from
both functions had no meaning.

Closes #4081
  • Loading branch information
SeanTAllen authored Nov 22, 2022
1 parent cfe1af0 commit ccd270b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 45 deletions.
15 changes: 15 additions & 0 deletions .release-notes/4081.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
## Improve TCP Backpressure on Windows

Our prior setting of backpressure for TCP writes on Windows was naive. It was based purely on the number of buffers currently outstanding on an IOCP socket. The amount of data didn't matter at all. Whether more data could be accepted or not, also wasn't taken into consideration. We've enhanced the backpressure support at both the Pony level in `TCPConnection` and in the runtime API.

Two runtime API methods have been updated on Windows.

### pony_os_writev

The Windows version of `pony_os_writev` will now return the number of buffers accepted or zero if backpressure is encountered. All other errors still cause an error that must be handled on the Pony side of the API via a `try` block.

### pony_os_send

The Windows version of `pony_os_send` will now return the number of bytes accepted or zero if backpressure is encountered. All other errors still cause an error that must be handled on the Pony side of the API via a `try` block.

The changes are considered non-breaking as previously, the return values from both functions had no meaning.
29 changes: 13 additions & 16 deletions packages/net/tcp_connection.pony
Original file line number Diff line number Diff line change
Expand Up @@ -472,18 +472,16 @@ actor TCPConnection is AsioEventNotify
end

// Write as much data as possible.
var len =
// Returns how many we sent or 0 if we are experiencing backpressure
let len =
@pony_os_writev(_event,
_pending_writev_windows.cpointer(_pending_sent),
num_to_send) ?
num_to_send)?

_pending_sent = _pending_sent + num_to_send.usize()

if _pending_sent > 32 then
// If more than 32 asynchronous writes are scheduled, apply
// backpressure. The choice of 32 is rather arbitrary an
// probably needs tuning
if len == 0 then
_apply_backpressure()
else
_pending_sent = _pending_sent + len
end
end
else
Expand Down Expand Up @@ -699,16 +697,15 @@ actor TCPConnection is AsioEventNotify
_pending_writev_windows .> push((data.size(), data.cpointer()))
_pending_writev_total = _pending_writev_total + data.size()

@pony_os_writev(_event,
_pending_writev_windows.cpointer(_pending_sent), I32(1)) ?
// Write as much data as possible
// Returns how many we sent or 0 if we are experiencing backpressure
let len = @pony_os_writev(_event,
_pending_writev_windows.cpointer(_pending_sent), I32(1))?

_pending_sent = _pending_sent + 1

if _pending_sent > 32 then
// If more than 32 asynchronous writes are scheduled, apply
// backpressure. The choice of 32 is rather arbitrary an
// probably needs tuning
if len == 0 then
_apply_backpressure()
else
_pending_sent = _pending_sent + len
end
end
else
Expand Down
62 changes: 33 additions & 29 deletions src/libponyrt/lang/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -371,28 +371,6 @@ static bool iocp_accept(asio_event_t* ev)
return true;
}

static bool iocp_send(asio_event_t* ev, const char* data, size_t len)
{
SOCKET s = (SOCKET)ev->fd;
iocp_t* iocp = iocp_create(IOCP_SEND, ev);
DWORD sent;

WSABUF buf;
buf.buf = (char*)data;
buf.len = (u_long)len;

if(WSASend(s, &buf, 1, &sent, 0, &iocp->ov, NULL) != 0)
{
if(GetLastError() != WSA_IO_PENDING)
{
iocp_destroy(iocp);
return false;
}
}

return true;
}

static bool iocp_recv(asio_event_t* ev, char* data, size_t len)
{
SOCKET s = (SOCKET)ev->fd;
Expand Down Expand Up @@ -945,14 +923,20 @@ PONY_API size_t pony_os_writev(asio_event_t* ev, LPWSABUF wsa, int wsacnt)

if(WSASend(s, wsa, wsacnt, &sent, 0, &iocp->ov, NULL) != 0)
{
if(GetLastError() != WSA_IO_PENDING)
switch (GetLastError())
{
iocp_destroy(iocp);
pony_error();
case WSA_IO_PENDING:
return wsacnt;
case WSAEWOULDBLOCK :
return 0;
default:
iocp_destroy(iocp);
pony_error();
return 0;
}
}

return 0;
return wsacnt;
}
#else
PONY_API size_t pony_os_writev(asio_event_t* ev, const struct iovec *iov, int iovcnt)
Expand All @@ -974,10 +958,30 @@ PONY_API size_t pony_os_writev(asio_event_t* ev, const struct iovec *iov, int io
PONY_API size_t pony_os_send(asio_event_t* ev, const char* buf, size_t len)
{
#ifdef PLATFORM_IS_WINDOWS
if(!iocp_send(ev, buf, len))
pony_error();
SOCKET s = (SOCKET)ev->fd;
iocp_t* iocp = iocp_create(IOCP_SEND, ev);
DWORD sent;

return 0;
WSABUF b;
b.buf = (char*)buf;
b.len = (u_long)len;

if(WSASend(s, &b, 1, &sent, 0, &iocp->ov, NULL) != 0)
{
switch (GetLastError())
{
case WSA_IO_PENDING:
return len;
case WSAEWOULDBLOCK :
return 0;
default:
iocp_destroy(iocp);
pony_error();
return 0;
}
}

return sent;
#else
ssize_t sent = send(ev->fd, buf, len, 0);

Expand Down

0 comments on commit ccd270b

Please # to comment.