wire: add a function to trunctate an ACK frame#5476
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a Truncate method for ACK frames to optimize packet packing by allowing explicit size-based truncation before serialization, rather than computing size on every append. The change increases MaxNumAckRanges from 32 to 64 (still fitting in a single-byte varint) and removes the MaxAckFrameSize constant, shifting responsibility for size limits to the caller via the new Truncate method.
Key changes:
- Adds
AckFrame.Truncate()method with fast-path optimization for small frames - Updates
MaxNumAckRangesfrom 32 to 64 to enable single-byte varint encoding - Simplifies loop constructs and optimizes
Length()calculation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/protocol/params.go |
Removes MaxAckFrameSize constant and increases MaxNumAckRanges to 64 with updated documentation explaining single-byte varint constraint |
internal/wire/ack_frame.go |
Adds Truncate() method, refactors numEncodableAckRanges() to accept size parameter, optimizes Length() calculation, simplifies loops, and fixes comment typo (NACK→ACK) |
internal/wire/ack_frame_test.go |
Replaces size limit test with comprehensive truncation tests covering single/multiple ranges with/without ECN, adds truncation to benchmarks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1ea4244 to
d854395
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d854395 to
c620f04
Compare
It is expected that when packing an ACK frame, the packer calls
Truncateto make sure that the ACK fits into the packet. This is faster, since we can skip the length calculation for small ACKs / large packet sizes.