Add a waiter box for the connection pool#397
Uh oh!
There was an error while loading. Please reload this page.
Conversation
| let id: ID | ||
| let request: HTTPScheduledRequest | ||
| let eventLoopRequirement: EventLoop? |
There was a problem hiding this comment.
In general for good hygiene I'd recommend these being var instead of let.
| let request: HTTPScheduledRequest | ||
| let eventLoopRequirement: EventLoop? | ||
| init(request: HTTPScheduledRequest, eventLoopRequirement: EventLoop?){ |
There was a problem hiding this comment.
Is there a reason we take the preference separately from the HTTPScheduledRequest, which encodes a preference?
fabianfettforce-pushed the ff-add-waiter branch 2 times, most recently from 0266fef to f92aa7aCompare
| extension HTTPConnectionPool{ | ||
| struct Waiter{ | ||
| struct ID: Hashable{ |
| var id: ID{ | ||
| ID(self.request) | ||
| } | ||
| var request: HTTPScheduledRequest{ | ||
| didSet{ | ||
| self.updateEventLoopRequirement() | ||
| } | ||
| } |
There was a problem hiding this comment.
Hmm, this feels a bit off. If we can the request then the id of the waiter also changes. Is that the behaviour we actually want or should it really be called requestID?
| struct RequestID: Hashable{ | ||
| private let objectIdentifier: ObjectIdentifier | ||
| init(_ request: HTTPScheduledRequest){ | ||
| self.objectIdentifier = ObjectIdentifier(request) | ||
| } | ||
| } | ||
| struct Waiter{ | ||
| var requestID: RequestID{ | ||
| RequestID(self.request) | ||
| } | ||
| var request: HTTPScheduledRequest{ | ||
| didSet{ | ||
| self.updateEventLoopRequirement() | ||
| } | ||
| } |
There was a problem hiding this comment.
@glbrntt Changed to RecordID. But I moved the RecordID type out of the waiter.
There was a problem hiding this comment.
This still says RequestID. Have I missed something?
There was a problem hiding this comment.
It used to be called ID and was on the type Waiter, but dependent on the HTTPScheduledRequest
There was a problem hiding this comment.
I see, I was confused by this comment saying the change was to RecordID.
| } | ||
| var request: HTTPScheduledRequest{ | ||
| didSet{ |
There was a problem hiding this comment.
Might it be easier to make eventLoopRequirement a computed property?
There was a problem hiding this comment.
Since HTTPScheduledRequest is a protocol, I wanted to keep the number of calls going through the lookup table as small as possible.
3 participants
Motivation
For the new
HTTPConnectionPool.StateMachinewe need a newWaitertype.Modifications
HTTPConnectionPool.Waiter