Skip to content

perf(net): avoid redundant chunk copies and plan performance investigations - #3443

Merged
kixelated merged 2 commits into
mainfrom
codex/performance-survey
Sep 5, 2026
Merged

perf(net): avoid redundant chunk copies and plan performance investigations#3443
kixelated merged 2 commits into
mainfrom
codex/performance-survey

Conversation

@kixelated

@kixelated kixelated commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Remove the intermediate incoming-chunk copy when the JS Reader joins buffered data. The combined buffer already owns a copy, so the extra allocation adds work without improving isolation. Add coverage for subarray offsets, input mutation, and retained output views.
  • Add six scoped quests for fragmented reader buffering, shared traffic-counter contention, CMAF copies, CPU/allocation profiling, repeatable benchmark comparisons, and browser benchmarks. Each names source evidence, workloads, metrics, and acceptance criteria.

A local allocation probe assembling 1 MiB from sixteen 64 KiB chunks dropped from 31 to 16 payload allocations, removing 960 KiB of temporary copying. Wall-clock samples were too noisy to claim a speedup.

Public API changes

None. No package versions change.

Wire behavior changes

None. Decoded bytes, buffering ownership, errors, and delivery semantics are preserved. Rust/API/spec synchronization is unnecessary for this internal JS allocation change.

Test plan

  • just fix origin/main, just check origin/main, and just test default origin/main passed locally using Nix-provided Bun 1.3.13. The full Nix shell was unavailable due to stalled startup; CI covers the complete pinned toolchain.
  • The targeted net suite passed 508 tests on the main-base code; subsequent changes only correct quest references and clarify overlap with existing work.
  • Quest validation passed all 253 documents. git diff --check origin/main...HEAD passed.
  • CI Check and Test passed on final head 18b1f6fa5. The local test recipe passed 1,100 tests.

(written by GPT-6)

…ations

Co-Authored-By: GPT-6 <noreply@openai.com>
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 822190a0-e835-42ed-a147-273a494a2c2b

📥 Commits

Reviewing files that changed from the base of the PR and between 84ee98b and 18b1f6f.

📒 Files selected for processing (9)
  • js/net/src/stream.test.ts
  • js/net/src/stream.ts
  • quest/m2/README.md
  • quest/m2/browser-benchmarks.md
  • quest/m2/cmaf-copy-budget.md
  • quest/m2/performance-comparisons.md
  • quest/m2/performance-profiles.md
  • quest/m2/stats-contention.md
  • quest/m2/stream-buffering.md

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.


Walkthrough

The change updates Reader.#fill to avoid an eager copy when its internal buffer is empty. A test verifies chunk ownership and returned-view isolation. The m2 quest index adds six performance entries. New quest documents define plans for benchmark comparison, relay profiling, browser benchmarks, stream buffering, traffic-counter contention, and CMAF copy measurement.

Merge Risk: ⚪ Minimal · up to 18b1f

This change reduces temporary Reader copying while preserving buffer isolation through targeted tests, and adds performance-planning documentation. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes both main changes: removing redundant network chunk copies and adding performance investigation plans.
Description check ✅ Passed The description directly explains the Reader allocation change, test coverage, added quests, measured results, and validation status.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch codex/performance-survey

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kixelated

Copy link
Copy Markdown
Collaborator Author

@codex review

(written by GPT-6)

@kixelated

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

(written by GPT-6)

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

@kixelated I will perform a complete review of PR #3443.

⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 12 minutes.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T06:58:08.248782Z 18b1f6f Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 414d232cf1

ℹ️ 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".

Comment thread quest/m2/performance-profiles.md Outdated
Co-Authored-By: GPT-6 <noreply@openai.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review head 18b1f6f, which corrects the quest paths reported in the previous review.

(written by GPT-6)

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 18b1f6fa52

ℹ️ 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".

@kixelated

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

Retrying after the stated quota reset. Please review head 18b1f6f; the benchmark and profiler quest references now match main.

(written by GPT-6)

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

@kixelated I will perform a complete review of PR #3443 at head 18b1f6fa5.

✅ Action performed

Full review finished.

@kixelated
kixelated merged commit f823668 into main Sep 5, 2026
2 checks passed
@kixelated
kixelated deleted the codex/performance-survey branch September 5, 2026 07:05
steelhead99x added a commit to steelhead99x/moq that referenced this pull request Sep 5, 2026
* docs(quest): settle scope narrowing in place, and mark pre-media sidecar placement (moq-dev#3427)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

* fix(moq-video): pick the V4L2 mode nearest the requested resolution (moq-dev#3355)

Co-authored-by: Luke Curley <kixelated@gmail.com>
Co-authored-by: Codex <codex@openai.com>

* feat(moq-video): add the Android MediaCodec encoder and decoder (moq-dev#3354)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Luke Curley <kixelated@gmail.com>

* docs(quest): import the post-grooming issues as quests (moq-dev#3431)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

* docs(quest): apply the Codex findings on the issue import (moq-dev#3432)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

* fix(claude): adopt a quest branch at the remote tip that was inspected (moq-dev#3421)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* docs(quest): record four findings from the m1 quest wave (moq-dev#3424)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: reorganize the site around what a reader can do (moq-dev#3426)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

* chore: ignore Claude Code's scratch directories (moq-dev#3428)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(moq-audio,moq-cli): assert publish_capture stays Send off macOS (moq-dev#3433)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* docs: correct claims found during merge review (moq-dev#3435)

Co-authored-by: GPT-5 <noreply@openai.com>

* docs(quest): import the open issues that had no quest, and gate the dev merge (moq-dev#3434)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

* docs(moq-audio): scope the local-task guidance to macOS (moq-dev#3436)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* docs: track deferred review findings (moq-dev#3438)

Co-authored-by: GPT-5 <noreply@openai.com>

* chore: remove redundant packaging work and plan relay ownership fixes (moq-dev#3440)

Co-authored-by: GPT-6 <noreply@openai.com>

* perf(net): avoid redundant chunk copies and plan performance investigations (moq-dev#3443)

Co-authored-by: GPT-6 <noreply@openai.com>

* fix(transcode): follow a source resolution change with the ladder (moq-dev#3381)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: GPT-5 <noreply@openai.com>

* feat(watch): share one AudioContext across audio decoders

Spatial playback needs every remote in the same Web Audio graph. Injected
contexts are never closed.

Co-Authored-By: Cursor Grok 4.6 <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Luke Curley <kixelated@gmail.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Franz Heinzmann <frando@unbiskant.org>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: Cursor Grok 4.6 <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant