ByteStream: hand off owned buffers from on_pull/on_data and skip the adapter pull view - #36736
Open
robobun wants to merge 15 commits into
Open
ByteStream: hand off owned buffers from on_pull/on_data and skip the adapter pull view#36736robobun wants to merge 15 commits into
robobun wants to merge 15 commits into
Claude / Claude Code Review
completed
Aug 1, 2026 in 28m 0s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/jsc/bindings/webcore/streams/BunStreamSource.cpp:634-635 |
Drain-first fast path is redundant for m_sourceOwnsChunks — extra native round-trip per pull and one extra pull to close |
Annotations
Check warning on line 635 in src/jsc/bindings/webcore/streams/BunStreamSource.cpp
claude / Claude Code Review
Drain-first fast path is redundant for m_sourceOwnsChunks — extra native round-trip per pull and one extra pull to close
The `|| adapter->m_sourceOwnsChunks` disjunct is redundant: for a ReadyOwned source `pendingView()` is provably always null, so without the disjunct control just falls through to `pull()` — and `ByteStream::on_pull` already fully subsumes `drain` (buffer non-empty → `Owned`/`OwnedAndDone` which also sets `closer[0]=true`; buffer empty → `Pending`/`Done`). Keeping the disjunct costs one extra `invokeMethod(drain)` round-trip per pull in the common parked-pull case, and one extra pull to observe c
Loading