perf: skip the per-stream and per-media walks in poll_event

[theovil1]

The problem

On every call to poll_output(), poll_event walks every stream and every media to ask whether anything is queued: four values_mut().find_map(..) walks (keyframe, sender feedback, REMB, paused), a loop over medias for MediaAdded and MediaChanged, and the sample walk in poll_event_fallible.

In a 1:1 call, with two m-lines, this is invisible. In a conference it is not: each Rtc holds roughly one m-line per remote participant, so the walks are O(N).

We run str0m server side in an SFU. In a 30 participant room, one connection makes about 53,000 poll_output() calls per second, and 86% of them return only a Timeout. Each of those is a full walk that finds nothing. The cost is paid O(N) times per iteration, O(N^2) per room, and it does not depend on how much media is actually flowing.

This is the same thing @lherman-cs described in #922 ("str0m is burning quite a few CPU cycles just polling for notifications ... ideally we'd have an O(1) way to handle stream notifications"), and the idea below is his.

The change

A single coarse readiness flag owned by Session:

#[derive(Debug, Default)]
pub(crate) struct Readiness(bool);

No shared mutable state, no atomic. The paths that queue an event take &mut Readiness and mark it where they already queue the thing: incoming PLI, FIR and REMB on StreamTx, a fresh sender report on StreamRx, pause and unpause transitions, the depayloader completing a sample, and the SDP paths that queue MediaAdded and MediaChanged.

The walks only run while the flag is marked, and it is cleared once an exhaustive poll has come back empty.

Correctness does not depend on the mark being tight. A spurious mark costs one extra walk, never a missed event. That is what makes it safe to keep the producer side conservative.

Two subtleties, since that is where the correctness lives:

• The mark is cleared at the end of the poll chain, not at the end of poll_event(). lib.rs calls poll_event() and then poll_event_fallible(), so the sample walk still has to run. In rtp_mode nothing walks the medias, so there the mark is spent in poll_event().
• It has to survive the pre SRTP window. poll_event() bails out early until SRTP is ready, while MediaAdded and MediaChanged are queued during negotiation, so clearing then would lose them.

This targets timeout-scheduler (#1008), which already removes the timeout side of the problem. No public API change.

Results

Our SFU, 100 participants in rooms of 10 on one thread, every camera on, one speaker at a time rotating with a 600 ms handover overlap, silent participants on Opus DTX. Same binary, only the str0m dependency changes. Paired runs, alternating order, 60 second windows.

CPU goes from 0.720 to 0.602 cores. Mean saving 16.1 percent, median 17.4, 6 of 6 pairs positive, 95 percent confidence interval 10 to 22.

The profile agrees from the other side:

symbol	#1008 alone	#1008 + this
Session::poll_event_fallible	6.8 %	1.5 %
Rtc::do_poll_output	6.0 %	2.1 %
Streams::poll_remb_request	2.0 %	0.6 %
Scheduler::arm	1.3 %	1.2 %

846 tests pass, clippy is clean.

Why one coarse bit is enough

I expected it to fail. We do not run in rtp_mode, so every incoming RTP packet depayloads and marks readiness, and I assumed the bit would end up permanently set and the guards useless.

It does not, because a mark only survives one exhaustive poll. At roughly 100 packets per second per client against thousands of poll_output calls, the vast majority of calls still skip the walks. Splitting readiness by event kind buys nothing measurable here, so this keeps the single bit.

---

Note: this PR was written in French and translated to English with the help of an AI tool. Apologies for any awkward phrasing.

[algesten]

@theovil1

Thanks for putting this together. The benchmark and the SFU numbers are very useful.

There is quite a bit of overlap with #1008. That PR removes the timeout scans for regular feedback, queued sends and packetization using the new scheduler. I think that makes EV_SEND, EV_PACKETIZE and feedback_at_cache redundant once #1008 lands.

The remaining poll_event() scans are still interesting. I'd like to keep that part, but not with Arc<AtomicU8>. str0m deliberately doesn't use shared mutable state internally. Everything is singly owned and mutation flows through &mut.

Can you rework this on top of #1008 (or soon merged to main), with readiness owned by Session. Event-producing paths get &mut Readiness (or just the relevant mutable field) and mark it when they actually queue an event. Session::poll_event() keeps the readiness set while it returns events, and clears it only after an exhaustive poll returns None.

That should give us the important fast path without the atomic or the timer-related parts of this PR. A single coarse readiness bit might be enough. If the benchmark says otherwise we can split it by event kind afterwards.

Since this changes the shape of the PR quite a bit, are you happy to rework it in that direction after #1008? We can also push commits to your branch if you'd rather we help with the rewrite.

[theovil1]

Thanks for the detailed read, and for taking the time.

Agreed on all three points. Dropping EV_SEND, EV_PACKETIZE and feedback_at_cache makes sense once #1008 lands, they clearly overlap with the scheduler. And the Arc<AtomicU8> was the wrong shape for str0m, I take the point about singly owned state, readiness on Session with &mut is cleaner than what I did.

Happy to rework it in that direction. I will rebase on timeout-scheduler and keep only the poll_event() part, with a single coarse readiness bit owned by Session, set by the paths that actually queue an event and cleared after an exhaustive poll returns None.

I will also run it on our SFU bench and report back on your open question, whether the coarse bit is enough or whether splitting by event kind buys anything measurable. That is the part I can answer with real numbers rather than an opinion.

No need to push commits to the branch, I will do the rewrite, but thanks for the offer.

[theovil1]

Reworked as you asked, on top of #1008. I retargeted the base to timeout-scheduler so the diff shows only my commits.

What changed from the first version. EV_SEND, EV_PACKETIZE and the feedback_at cache are gone, the scheduler covers all of it. No more Arc<AtomicU8>: readiness is a plain bool newtype owned by Session, and the producing paths take &mut Readiness. Those are incoming PLI, FIR and REMB on StreamTx, a fresh sender report on StreamRx, pause and unpause transitions, the depayloader completing a sample, and the SDP paths that queue MediaAdded and MediaChanged. A single coarse bit, as you suggested. It turns out to be enough.

Two things worth flagging, since that is where the correctness lives. The mark is cleared at the end of the poll chain rather than at the end of poll_event(), because lib.rs calls poll_event() and then poll_event_fallible(), so the sample walk still has to run. In rtp_mode nothing walks the medias, so there the mark is spent in poll_event(). And it has to survive the pre SRTP window: poll_event() bails out early until SRTP is ready, while MediaAdded and MediaChanged are queued during negotiation, so clearing then would lose them.

846 tests pass, clippy is clean.

Numbers on our SFU: 100 participants in rooms of 10 on one thread, every camera on, one speaker at a time rotating with a 600 ms handover overlap, silent participants on Opus DTX. Same binary, only the str0m dependency changes. Paired runs, alternating order, 60 second windows. CPU goes from 0.720 to 0.602 cores. Mean saving 16.1 percent, median 17.4, 6 of 6 pairs positive, 95 percent confidence interval 10 to 22.

The profile agrees from the other side, and this is what convinced me the coarse bit is enough:

symbol	#1008 alone	#1008 + this
Session::poll_event_fallible	6.8 %	1.5 %
Rtc::do_poll_output	6.0 %	2.1 %
Streams::poll_remb_request	2.0 %	0.6 %
Scheduler::arm	1.3 %	1.2 %

I expected the coarse bit to fail, actually. We do not run in rtp_mode, so every incoming RTP packet depayloads and marks readiness, and I assumed the bit would end up permanently set. It does not, because a mark only survives one exhaustive poll: at roughly 100 packets per second per client against thousands of poll_output calls, the vast majority still skip the walks. Splitting by event kind buys nothing measurable here, so I would keep the single bit.

[theovil1]

All six applied, thanks.

handle_rtcp now takes one bind for the function, and poll_event reuses a single let ready = self.event_ready.is_marked() across both guards. The two update_register call sites bind locally too. One of them still wraps: with is_repair in the argument list the line lands at 103 characters, so rustfmt breaks it whatever I name the bind. The other one now fits on a single line.

I also fixed the CI, which was my fault twice over. The readiness guard used a let chain, which is only stable from 1.88, so it broke the 1.85 MSRV job, and I had not run rustfmt. Both are fixed, and I folded the MSRV fix into the commit that introduced it rather than adding a fix on top, so every commit in the branch now passes cargo +1.85.0 check --all-targets and cargo fmt --check on its own. Verified commit by commit.

846 tests pass, clippy clean.

[algesten]

@theovil1 This should not been closed! 🤦

Can you reopen against main?