Add a RequestQueue#412
Add a RequestQueue #412
Uh oh!
There was an error while loading. Please reload this page.
Conversation
fabianfett added 🔨 semver/patch 
fabianfettforce-pushed the ff-pool-state-queue branch from ddc0233 to 77809ccCompare
glbrntt left a comment
glbrntt left a comment There was a problem hiding this comment.
I think the queue has some surprising behaviour that should be changed or at least made much clearer.
Uh oh!
There was an error while loading. Please reload this page.
| self.generalPurposeQueue.isEmpty | ||
| } | ||
| mutatingfunc count(for eventLoop:EventLoop?)->Int{ |
There was a problem hiding this comment.
I think as a user I'd be very surprised that getting the count is mutating and potentially allocates!
Perhaps there should be a non-mutating version of withEventLoopQueue, something like:
func withEventLoopQueueIfAvailable<Result>( for eventLoopID:EventLoopID, _ closure:(CircularBuffer<Request>)->Result)->Result?| returnself.generalPurposeQueue.count | ||
| } | ||
| mutatingfunc isEmpty(for eventLoop:EventLoop?)->Bool{ |
| mutatingfunc removeAll()->[Request]{ | ||
| varresult=[Request]() | ||
| result =self.eventLoopQueues.reduce(into: result){ partialResult, element in |
There was a problem hiding this comment.
Isn't this just eventLoopQueues.flatMap{$1 }?
| self.generalPurposeQueue.forEach{ request in | ||
| result.append(request) | ||
| } |
There was a problem hiding this comment.
Use result.append(contentsOf: self.generalPurposeQueue) to avoid intermediate reallocs of result
Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+RequestQueue.swiftShow resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
| import NIOEmbedded | ||
| /// An `EventLoopGroup` of `EmbeddedEventLoop`s. | ||
| finalclassEmbeddedEventLoopGroup:EventLoopGroup{ |
| varcount:Int{ | ||
| self.generalPurposeQueue.count | ||
| } | ||
| varisEmpty:Bool{ | ||
| self.generalPurposeQueue.isEmpty | ||
| } |
There was a problem hiding this comment.
This is surprising behaviour I think -- I would expect the count to return the total count and isEmpty to only return true if both queues are empty.
| } | ||
| } | ||
| func __testOnly_internal_value()->HTTPSchedulableRequest{ |
There was a problem hiding this comment.
nit: "wrapped request" or similar would be clearer than internal_value
glbrntt left a comment
glbrntt left a comment There was a problem hiding this comment.
Looks good modulo a couple of nits
| self.count =0 | ||
| } | ||
| private(set)varcount:Int |
There was a problem hiding this comment.
How often is this used externally? If it's not used frequently then dynamically computing this would be better I think...
| ifself.eventLoopQueues[eventLoopID]!=nil{ | ||
| returnclosure(self.eventLoopQueues[eventLoopID]!) |
There was a problem hiding this comment.
nit: avoid hashing twice here (also we can avoid the bang)
2 participants
Motivation
For our Connection Pool we need to queue requests.
Changes
Waitertype... Not needed anymore.RequestboxRequestQueueResult
Less code in the actual state machine.