Use NIO scheduleCallback API for connection handler timers #28
Conversation
| extension ClientConnectionHandler { | ||
| struct LoopBoundView: @unchecked Sendable { | ||
| final class MaxIdleTimerHandlerView: @unchecked Sendable, NIOScheduledCallbackHandler { |
There was a problem hiding this comment.
Couple of questions:
- Does this need to be a
class? - Does it need to be
Sendable? The previousLoopBoundViewwasSendablebecause it needed to be pass to theschedule(on:)closure, I don't think we have a similar constraint any more (?)
If it doesn't need to be Sendable then I think we can remove the assertInEventLoop checks as well.
There was a problem hiding this comment.
- Does this need to be a
class?
These view types can be structs. That they are classes, is a hangover from before I pushed this logic into Timer. Timer itself needs to be a class because the protocol requirement for handleScheduledCallback is non-mutating.
I've added a commit to address that.
- Does it need to be
Sendable? The previousLoopBoundViewwasSendablebecause it needed to be pass to theschedule(on:)closure, I don't think we have a similar constraint any more (?)
It wasn't the case when the API was added to NIO, but it looks like it got changed since and now it is a requirement that the handler be Sendable:
@preconcurrency
@discardableResult
func scheduleCallback(
in amount: TimeAmount,
handler: some (NIOScheduledCallbackHandler & Sendable)
) throws -> NIOScheduledCallback| package func handleScheduledCallback(eventLoop: some EventLoop) { | ||
| self.eventLoop.assertInEventLoop() | ||
| self.handler.handleScheduledCallback(eventLoop: eventLoop) | ||
| if self.repeating { self.start() } |
There was a problem hiding this comment.
Can you remind me what happens w.r.t. the callback if it's cancelled? Just wondering if it works how we expect for the repeating timers.
There was a problem hiding this comment.
Note that the NIO scheduled callback API is always one-shot. The repeated timer API here is layered on top.
The semantics of this timer is that if you cancel it you cancel all future recurrences too, but it can then be reused/restarted by calling start again.
There's a test case that covers this behaviour.
| loop.advanceTime(by: .milliseconds(999)) | ||
| XCTAssertEqual(value.load(ordering: .acquiring), 0) | ||
| XCTAssertEqual(handler.counter.value, 0) |
There was a problem hiding this comment.
Completely optional: but feel free to write these using swift-testing (we're slowly rolling our tests over to it). No problem if you don't want to spend the time doing it.
There was a problem hiding this comment.
Oh, forgot that was an option. I'll pass on it this time, but I'll remember it for next time.
Motivation
NIO 2.75.0 added new APIs for scheduling callbacks, which can be used to implement timers with fewer allocations than the previous APIs1.
Modifications
Replace the timers used in the client and server connection handlers with a new implementation that uses these new NIO APIs.
Result
This should have no functional impact. However it reduces the allocations in the connection handler. How many allocations it reduces will depend on the exact scenario, but running the
echoserver under Instruments and sending 1000 requests suggests around 10%.Footnotes
https://github.com/apple/swift-nio/releases/tag/2.75.0 ↩