Add a waiter box for the connection pool #397
Conversation
fabianfett
added
the
semver/patch
|
|
||
| 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?
fabianfett
force-pushed
the
ff-add-waiter
branch
2 times, most recently
from
0266fef
to
f92aa7a
Compare
|
|
||
| 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.
Motivation
For the new
HTTPConnectionPool.StateMachinewe need a newWaitertype.Modifications
HTTPConnectionPool.Waiter