Skip to content

Reduce TWCC sender allocations#426

Merged
kcaffrey merged 1 commit into
pion:mainfrom
kcaffrey:reduce-allocations
Jul 21, 2026
Merged

Reduce TWCC sender allocations#426
kcaffrey merged 1 commit into
pion:mainfrom
kcaffrey:reduce-allocations

Conversation

@kcaffrey

Copy link
Copy Markdown
Contributor

Recorder.Record heap-allocates on every call: it stores the unwrapped sequence number into startSequenceNumber by taking the address of a local, which moves the local to the heap whether or not the branch is taken. Since Record runs once per received RTP packet, this costs one allocation per packet. Updates now go through a setter that writes into a single reused allocation.

feedback.addReceived also allocated one *rtcp.RecvDelta per received packet. Deltas are now accumulated by value, and the []*rtcp.RecvDelta that the rtcp API requires is materialized once per feedback packet in getRTCP, which hands ownership of the backing array to the packet.

No exported API or behavior changes.


Benchmarks (added in this PR, run on main vs this branch on a M2 MacBook Air; the second records 100 packets per feedback):

main this PR
BenchmarkRecord 21.1 ns/op, 1 alloc/op 6.9 ns/op, 0 allocs/op
BenchmarkRecordAndBuildFeedbackPacket 4546 ns/op, 322 allocs/op 2270 ns/op, 21 allocs/op

TestRecordDoesNotAllocateInSteadyState pins the steady-state Record path at zero allocations.

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.09%. Comparing base (c927aa2) to head (34e99f0).

Files with missing lines Patch % Lines
pkg/twcc/twcc.go 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #426      +/-   ##
==========================================
- Coverage   80.13%   80.09%   -0.04%     
==========================================
  Files          88       88              
  Lines        4617     4623       +6     
==========================================
+ Hits         3700     3703       +3     
- Misses        735      736       +1     
- Partials      182      184       +2     
Flag Coverage Δ
go 80.09% <92.30%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoTurk JoTurk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thank you

Record stored the unwrapped sequence number by taking its address,
which heap-allocates the local on every call, whether or not the
branch is taken. Updates now go through a setter that writes into a
single reused allocation.

addReceived allocated one rtcp.RecvDelta per received packet. Store
values and build the pointer slice once per feedback in getRTCP.

BenchmarkRecord: 21.1ns/op 1 alloc -> 6.9ns/op 0 allocs.
BenchmarkRecordAndBuildFeedbackPacket (100 packets per feedback):
4546ns/op 322 allocs -> 2270ns/op 21 allocs.
@kcaffrey
kcaffrey force-pushed the reduce-allocations branch from d6f41c2 to 34e99f0 Compare July 21, 2026 00:06
@kcaffrey
kcaffrey merged commit 76e40bb into pion:main Jul 21, 2026
18 checks passed
@kcaffrey
kcaffrey deleted the reduce-allocations branch July 21, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants