Skip to content

Reuse the sent packet deque across sendFrames calls - #58

Merged
tfpauly merged 2 commits into
apple:mainfrom
glbrntt:reuse-packet-deque
Jul 30, 2026
Merged

Reuse the sent packet deque across sendFrames calls#58
tfpauly merged 2 commits into
apple:mainfrom
glbrntt:reuse-packet-deque

Conversation

@glbrntt

@glbrntt glbrntt commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

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

@glbrntt

glbrntt commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

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
@glbrntt
glbrntt force-pushed the reuse-packet-deque branch from 1ca3472 to 32fe031 Compare July 28, 2026 14:22
// '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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a setting in Preferences.swift

@glbrntt
glbrntt requested a review from rpaulo July 29, 2026 07:08
}
return withCurrentPath { path in
var sentPackets = NetworkUniqueDeque<SentPacketRecord>(minimumCapacity: capacityForPacketNumberSpace())
self.sentPackets.reserveCapacity(capacityForPacketNumberSpace())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, if it only reallocates on insufficient capacity then we should be good.

@tfpauly
tfpauly merged commit 23502de into apple:main Jul 30, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants