Repository navigation
quest: audit memory retention and kernel settings - #6332
Conversation
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
# Conflicts: # quest/m2/README.md # quest/m2/performance-survey/README.md # quest/m2/performance-survey/quic-schedule-churn.md
The maintainer picked #6350's quest/m2/perf/uring-pool-memory.md over uring-pools; socket-queues names that PR until it merges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#5878 finished and deleted quest/m1/quic/receive-memory.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7405e7157
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| For settings sweeps, record effective values, not only requested ones. | ||
| [Resolved client defaults](/rs/moq-tokio/src/quic.rs) currently allow 10,000 | ||
| streams per direction, 64 MiB connection receive credit, 4 MiB per-stream |
There was a problem hiding this comment.
Update the client receive-window default to 128 MiB
At the reviewed commit, rs/moq-tokio/src/quic.rs:119 defines DEFAULT_RECEIVE_WINDOW as 128 << 20, not 64 MiB. Because this paragraph establishes the effective baseline for subsequent settings sweeps, the incorrect value could cause experiments to compare against or explicitly configure the wrong baseline; update it to 128 MiB.
AGENTS.md reference: AGENTS.md:L32-L32
Useful? React with 👍 / 👎.
Problem
The performance survey needs coverage for memory after bursts, growth with historical usage, idle browser work and effective kernel/runtime settings. Source inspection identified concrete retention paths, but their production cost has not been measured.
Approach
Add eight benchmark-first quests: recycled QUIC receive state, UDP pool recovery, cache metadata capacity, shared backing amplification, idle browser stats, transport preference history, resume identity history and UDP socket queue/drop attribution. Extend the existing worker-default quest with NUMA, SMT and NIC placement experiments.
Require burst/quiet/reuse/teardown measurements that distinguish live allocations, allocator resident/retained memory and kernel queues. Preserve equivalent useful work, paired measurements, browser and physical-NIC validation where applicable, and nightly benchmark coverage. A measured no-win is valid. Document client versus relay-peer limits, already-enabled io_uring features, existing kernel experiment ownership and previously rejected ideas.
This follows moq-dev/moq#6318, which entered the merge queue before this follow-up could be pushed. Its commits are included as prerequisites until it lands; merge this follow-up after it. The combined survey has 23 child quests.
Impact
Alternatives
Reuse existing receive-memory, QUIC I/O ownership, shard-default, keepalive and zero-copy-send quests. Do not duplicate the release audit or reopen busy polling without new profile evidence. Keep implementation and tuning decisions conditional on measurements.
Validation
just cipasses in the Nix environment.quest check --base origin/main: 467 documents pass.Follow-ups
The eight new quests and expanded worker-placement experiment are the requested follow-ups. No implementation contract or API choice is settled here.
(Written by GPT-6)