Reuse the sent packet deque across sendFrames calls - #58
Conversation
|
When running some http/3 benchmarks (100k requests, each a 1k download with 100 concurrent streams on one connection) this saves 100k allocs (only ~2% or so), but reduces total bytes allocated by ~15%. |
Motivation: Both sendFrames() overloads create a new deque of SentPacketRecord on every call, only to hand its contents to recovery and throw the storage away. Modifications: - Store a deque on the QUICConnection and reuse it. - Take 'sentPackets' inout rather than consuming in Recovery.recordSentPackets. It already drained the storage in place so now the connection can reuse the empty deque. Result: - Fewer allocations
1ca3472 to
32fe031
Compare
| // 'sentPackets' is held onto for the lifetime of the connection. If a send burst grows it | ||
| // beyond a certain limit then drop the capacity. This avoids bursty traffic bloating memory | ||
| // indefinitely. | ||
| if self.sentPackets.capacity > 512 { |
There was a problem hiding this comment.
This should be a setting in Preferences.swift
| } | ||
| return withCurrentPath { path in | ||
| var sentPackets = NetworkUniqueDeque<SentPacketRecord>(minimumCapacity: capacityForPacketNumberSpace()) | ||
| self.sentPackets.reserveCapacity(capacityForPacketNumberSpace()) |
There was a problem hiding this comment.
We should profile this and see if this does an explicit reallocation or it ONLY does a reallocation if it needs more capacity. I really don't know what the behavior is here. If it does a reallocation each time we should get rid of this if we are storing it at the connection level. Declaring a capacity up front should get us what we want.
There was a problem hiding this comment.
It only does a reallocation if there's insufficient capacity, and that path will be very (i.e. compare requested capacity with actual).
This avoids intermediate reallocations if the deque was shrunk in the previous round because it got too big.
There was a problem hiding this comment.
Okay, if it only reallocates on insufficient capacity then we should be good.
Motivation:
Both sendFrames() overloads create a new deque of SentPacketRecord on every call, only to hand its contents to recovery and throw the storage away.
Modifications:
Result: