Connection pool optimization: reduce connection maintenance loops complexity #929
Conversation
MarkusSintonen
changed the title
MarkusSintonen
force-pushed
the
optimize-connection-pool-loops
branch
2 times, most recently
from
da9636b to
b6fd02c
Compare
MarkusSintonen
force-pushed
the
optimize-connection-pool-loops
branch
from
6e4b9aa to
a8d638a
Compare
|
wonder if there are any further progress for this PR, our team are waiting for this fix @MarkusSintonen @T-256 |
|
@MarkLux I don't know where the author has gone :/ |
There was a problem hiding this comment.
Hi @MarkusSintonen! Thank you for your pr, I hope to see it merged soon.
Recently I've faced a performance issue in the pool, in case of a growing number of requests awaiting execution. It looks like your pr will partially solve such issues, I've marked a couple of places with potential improvements.
Also, have you thought about changing the List of _requests to something more optimal for queue tasks, for example, deque? It could reduce CPU usage for remove operations, especially in cases of huge queue.
| # log: "closing idle connection" | ||
| self._connections.remove(connection) | ||
| closing_connections.append(connection) | ||
|
|
||
| # Assign queued requests to connections. | ||
| queued_requests = [request for request in self._requests if request.is_queued()] | ||
| for pool_request in queued_requests: | ||
| for pool_request in list(self._requests): |
There was a problem hiding this comment.
Do we need a copy of the list here?
| idling_connection = next( | ||
| (c for c in self._connections if c.is_idle()), None | ||
| ) | ||
| if idling_connection is not None: |
There was a problem hiding this comment.
Should we continue iterating over self._requests if there is no available or idling connection? Can we break in such cases?
Summary
This is a small low hanging fruit for optimizing and simplifying the connection maintenance loops in the connection pool. This work is based on suggestion by @T-256 here (thanks!). Previously performance of
httpcoredegraded as connection count increased in the pool. As the maintenance loops had to do more and more work iterating and reiterating the connections.This optimization brings the performance of
httpcoreto same level asurllib3with sync-usage.The benchmarks below include the socket polling fix. (The socket polling problem makes request processing latency so highly varying that it overshadows everything.)
Previously with sync (with optimized socket polling):
PR with sync:
As it can be seen the performance gets to exact same level as
urllib3. There is almost no overhead fromhttpcorerelated request processing anymore.Checklist