Skip to content

perf(noq-proto): lazily allocate remote stream slots#667

Merged
matheus23 merged 1 commit into
n0-computer:mainfrom
kixelated:lazy-remote-streams
May 21, 2026
Merged

perf(noq-proto): lazily allocate remote stream slots#667
matheus23 merged 1 commit into
n0-computer:mainfrom
kixelated:lazy-remote-streams

Conversation

@kixelated

Copy link
Copy Markdown
Contributor

Port of quinn-rs/quinn#2601.

StreamsState::new used to eagerly insert (id, None) placeholder entries in both send and recv FxHashMaps for every remote stream id in 0..max_remote[dir]. With max_concurrent_*_streams = 10_000 (e.g. for MoQ relay nodes that burn through short-lived streams), hashbrown rounded each map's bucket array up to ~65K buckets, costing ~0.5-2 MB of bucket memory per Connection before any stream data was sent.

Port of quinn-rs/quinn#2601.

`StreamsState::new` used to eagerly insert `(id, None)` placeholder
entries in both `send` and `recv` FxHashMaps for every remote stream id
in `0..max_remote[dir]`. With `max_concurrent_*_streams = 10_000` (e.g.
for MoQ relay nodes that burn through short-lived streams), hashbrown
rounded each map's bucket array up to ~65K buckets, costing ~0.5-2 MB
of bucket memory per Connection before any stream data was sent.

Move that work into a lazy frontier-advance step at the top of each
remote receive path. Remote stream state:

- `id.index() >= max_remote[dir]`: not allowed
- `id.index() >= next_remote[dir]`: allowed, absent from map
- `id.index() <  next_remote[dir]`, present: active
- `id.index() <  next_remote[dir]`, absent: closed

RFC 9000 section 3.2 says receiving a frame for index N implicitly
opens all lower indices. `ensure_remote` handles that by materializing
`None` placeholders for every index in `[next_remote, id.index()]` -
otherwise a late out-of-order frame for a skipped index would land on
"absent from map" and get dropped as closed. Placeholders are
short-lived: they get filled on first real access or removed when the
stream closes, so unlike a growing "closed ids" set the map stays
bounded by currently live streams even across heavy churn.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@n0bot n0bot Bot added this to iroh May 20, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh May 20, 2026

@matheus23 matheus23 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.

FWIW we merge upstream Quinn into noq every now and then. So this would've made it into noq after getting merged into Quinn anyways.

The good news is that by submitting this here now you're probably getting this PR into an earlier release (we're probably releasing again on Monday).

@matheus23

Copy link
Copy Markdown
Member

I think the only failing CI is an action that wants to write a comment on this PR but can't because it was submitted from a fork or sth like that.

@matheus23 matheus23 added this pull request to the merge queue May 21, 2026
Merged via the queue into n0-computer:main with commit 4d29110 May 21, 2026
44 of 46 checks passed
@github-project-automation github-project-automation Bot moved this from 🚑 Needs Triage to ✅ Done in iroh May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants