warp/recv: optimistic non-blocking recv before parking the thread - #1099
warp/recv: optimistic non-blocking recv before parking the thread#1099seanparsons wants to merge 1 commit into
Conversation
The graceful-shutdown receive path (makeGracefulRecv) registered with
the IO manager and parked in STM on EVERY receive: waitReadSocketSTM
creates an event-manager registration up front, then the composed
atomically blocks until the socket is readable, and only then is
recv(2) called. Under load the next pipelined/keep-alive request is
almost always already in the kernel buffer, so every request paid one
epoll_ctl plus a futex sleep/wake pair for nothing. A syscall census
of a hello-world server showed ~1 epoll_ctl and ~28 futex calls per
request, with recv essentially never returning EAGAIN. Since
runSettings always sets up ServerState, every default-configured warp
server pays this.
Changes:
* recv: new receiveNoWait, a direct non-blocking recv(2) on the
already-non-blocking fd (one unsafe foreign call; the socket is
SOCK_NONBLOCK so it returns immediately). The new
tryWithBufferPool leaves the buffer pool intact when the filler
declines. EAGAIN and real errors both map to Nothing, deferring to
the blocking path so exceptions surface there with errno intact.
Just "" is EOF. Windows: always Nothing (unchanged behavior).
* warp: makeGracefulRecv tries receiveNoWait first and enters the
STM/shutdown machinery only on would-block. The slow path is
byte-for-byte the previous implementation. A request already in
the kernel buffer is now served even if shutdown began meanwhile,
which merely restores pre-graceful-shutdown behavior for bytes the
client already sent.
* The Conc.yield after each response is kept and its comment
updated: measurements show it is load-bearing in combination with
this fast path (it gives the next request time to arrive, raising
the fast-path hit rate; removing it costs ~17% throughput).
Syscall census after: 186 epoll_ctl and 725 futex calls TOTAL across
198k requests (previously ~1 and ~28 per request); the per-request
profile is one recvfrom + one sendto. Measured standalone-equivalent
on the full optimization series: 52.8k -> 71.7k req/s (stock warp
baseline 35.7k); unpinned -N1: 67.6k -> 91.5k req/s. Also validated
on a production Servant app: +57-80% on light endpoints with
byte-identical responses. Spec suite passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HBrMGfWRxTpYeEB4UT8Kf
kazu-yamamoto
left a comment
kazu-yamamoto
left a comment
There was a problem hiding this comment.
I support this change because it restores an optimization that was lost when transitioning to the event-poll model. The recv function in the network library is actually implemented this way: it first attempts a non-blocking c_recv and only invokes the I/O manager if it encounters EAGAIN.
|
@Vlix Compared to other PRs, I think this one is clear and easy to merge. |
So under load |
Vlix
left a comment
Vlix
left a comment
There was a problem hiding this comment.
This might break the graceful shutdown. We need to know the extent of this change before we can accept it.
| mbs <- receiveNoWait sock pool | ||
| case mbs of | ||
| Just bs -> return bs | ||
| Nothing -> slowPath |
There was a problem hiding this comment.
Doesn't this mean that we might never check the shutting down state if the client is prompt with their bytes and we happen to only recv when there's bytes in the kernel buffer?
| -- and produces 'Nothing'. | ||
| tryWithBufferPool | ||
| :: BufferPool -> (Buffer -> BufSize -> IO Int) -> IO (Maybe ByteString) | ||
| tryWithBufferPool (BufferPool l h ref) f = do |
There was a problem hiding this comment.
I don't like the duplication of code here, though we might have to.
This will also need tests like 'withBufferPool', though, if it IS duplicated.
And it will need tests either way, even if we refactor withBufferPool for this new functionality.
| return $ fromIntegral n | ||
|
|
||
| foreign import ccall unsafe "recv" | ||
| c_recv :: CInt -> Ptr CSsize -> CSize -> CInt -> IO CSsize |
There was a problem hiding this comment.
We basically want to use recvBufNoWait from Network.Socket.Buffer, but it's not exported.
There was a problem hiding this comment.
If necessary, I can export it from network.
There was a problem hiding this comment.
I think that in itself is a good idea. It's nice for the user to be able to decide to block or not block on a recv.
4 participants
Part of a series splitting #1090 into independently reviewable PRs; self-contained (recv + warp). The single largest win of the series.
makeGracefulRecv(on the hot path of everyrunSettingsserver, sinceServerStateis always set up) registers with the IO manager and parks in STM on every receive,waitReadSocketSTMcreates an event-manager registration up front, the composedatomicallyblocks until readable, and only then isrecv(2)called. Under load the next keep-alive request is nearly always already in the kernel buffer, so every request paid oneepoll_ctlplus a futex sleep/wake pair for nothing. Syscall census before: ~1epoll_ctland ~28futexcalls per request, with recv essentially never returning EAGAIN.Changes:
recvgainsreceiveNoWait(direct non-blockingrecv(2)on the already-SOCK_NONBLOCK fd;tryWithBufferPoolkeeps the pool intact on decline; EAGAIN and real errors both defer to the blocking path so exceptions keep their errno;Just ""is EOF; Windows always returnsNothing= unchanged behavior).makeGracefulRecvtries it first; the slow path is byte-for-byte the previous implementation. One edge: a request already in the kernel buffer is served even if shutdown began meanwhile restoring pre-graceful-shutdown behavior for bytes the client already sent. TheConc.yieldafter each response turns out to be load-bearing with this fast path (it lets the next request arrive; removing it costs ~17%), comment updated.After: 186
epoll_ctland 725futexcalls total across 198k requests; the per-request syscall profile is one recvfrom + one sendto. Measured: +36% on top of the rest of the series (52.8k → 71.7k req/s; stock baseline 35.7k), −N1 unpinned 67.6k → 91.5k. Also validated on a production Servant app: +57-80% on light endpoints, byte-identical responses. Spec suite passes.Note:
receiveNoWaituses its own FFIrecv(2)becauseNetwork.Socket.recvBufNoWaitisn't exported fromnetwork's public API; happy to switch if exporting it upstream is preferred.