Repository navigation
Hand EndpointFlow stream writes to the stack in batches - #279
Conversation
`EndpointFlow` has to order queued writes by priority without moving a higher-priority request ahead of ones it has already committed. A plain deque cannot say which of its front elements are committed. * Added `NetworkUniqueMarkedDeque`, a deque with a mark over its front elements that priority insertion never crosses. * Added `appendOrderedByPriority` for `WriteRequest`. * Added `SwiftNetworkMarkedDequeTests`.
`EndpointFlow` passed each write to the stack as it arrived, so a burst of writes paid the stack's per-call work (`serviceStreamDataToSend`, packet building, a completion hop) once per write. * Deferred the hand-off to one context task, `scheduleWriteFlush`, so writes queued in the same turn reach the stack together. * Replaced the per-request drain with `writeQueuedRequests`, which hands runs of requests over in one `FrameArray` through `writeFrameBatch`. * Made `writeRequests` a `NetworkUniqueMarkedDeque`, marking a request that has nothing uncommitted ahead of it so a later higher-priority request cannot overtake it. * Unmarked the queue when the stack has no room, so a backlog is still reordered by priority. * Added `StreamEndpointFlowProtocol.write(_:in:)` for a `FrameArray`. * Added ordering tests for mixed-priority writes submitted together.
agnosticdev
left a comment
agnosticdev
left a comment
There was a problem hiding this comment.
Cool, so if I am understanding correctly, we are batch draining the writes up to the mark and then when we do so we move the mark to the end / count of elements still waiting. Is that correct?
| for _ in 0..<requestCount { | ||
| guard let writeRequest = writeRequests.popFirst() else { | ||
| break | ||
| } |
There was a problem hiding this comment.
It "may" be more performant to do a while requestCount > 0 writeRequests.removeFirst()` type of pattern, but not blocking.
There was a problem hiding this comment.
I'm sorry, I'm not seeing this - what aspect of this do you hope to see a speed up for?
There was a problem hiding this comment.
Not blocking, I've just found removeFirst() to be more performant than popFirst()
Not quite, the drain isn't bounded by the mark. |
…atch `writeFrameBatch` kept each request's completion in an `Array` of optional closures. A closure stored in a generic container is converted to its most general representation, which allocates a thunk for every element. * Held the completions in an array of `BatchedWriteCompletion`, which keeps each closure in its natural representation. * Skipped scheduling the delivery task when no request in the batch has a completion. * Added tests for a batch where only some writes ask for a completion and a batch where none do.
61dec0a to
939d8b6
Compare
Batching made `EndpointFlow` queue write requests briefly before handing them to the stack, so `NetworkUniqueMarkedDeque` marked the requests that must not be reordered. That retained the behavior of a priority system that is currently unused, is still in development, and may change. Changed: * Queued write requests in `EndpointFlow` in submission order, in a `NetworkUniqueDeque`. * Removed `NetworkUniqueMarkedDeque`, its tests, and the marking in `addWriteRequestOnContext` and `write`. Result: * The batching change no longer adds a type or any ordering policy; writes keep the order they were submitted in.
…twork-evolution into batch-endpoint-flow-writes
# Conflicts: # Sources/SwiftNetwork/EndpointFlow/EndpointFlow.swift
EndpointFlowpassed each write to the stack as it arrived. A caller submitting many small writes in a row paid the stack's per-call work once per write: aserviceStreamDataToSend, a packet-building pass, and a completion hop. Writing larger pieces avoids that cost and gives higher throughput, but callers don't always control the piece size.Modifications:
scheduleWriteFlush. Writes queued in the same scheduler turn reach the stack together.writeQueuedRequests. A lone request, or one that fills the available room by itself, goes throughwriteSingleFrame. Any other run goes throughwriteFrameBatchas oneFrameArray, sized once up front.writeRequestsaNetworkUniqueMarkedDeque, added in the first commit. A request that arrives with nothing uncommitted ahead of it is marked, so a later request with a higher priority cannot overtake it.StreamEndpointFlowProtocol.write(_:in:)for aFrameArray.Writes are always flushed at the end of the turn now.
NOTE: At the moment a pre-existing issue means that the room never reaches zero, so batching works but doesn't bound the send queue. A separate PR (#282) makes a QUIC stream's reported send room follow its queued bytes. This PR doesn't depend on that change.