Repository navigation
socket: drain node:net's pending-write queue with a cursor #44219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
robobun
wants to merge
1
commit into
main
Choose a base branch
from
robobun/0fba4e9e/socket-write-queue-linear-drain
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| use bun_io::StreamBuffer; | ||
| use bun_jsc::{CallFrame, JSGlobalObject, JSValue, JsResult}; | ||
|
|
||
| /// Unsent bytes of node:net writes. A drain moves no bytes and an empty queue holds no allocation. | ||
| #[derive(Default)] | ||
| pub(crate) struct PendingWrites(StreamBuffer); | ||
|
|
||
| impl PendingWrites { | ||
| #[inline] | ||
| pub(crate) fn len(&self) -> usize { | ||
| self.0.size() | ||
| } | ||
|
|
||
| #[inline] | ||
| pub(crate) fn slice(&self) -> &[u8] { | ||
| self.0.slice() | ||
| } | ||
|
|
||
| /// The allocation, sent prefix included. | ||
| #[inline] | ||
| pub(crate) fn capacity(&self) -> usize { | ||
| self.0.memory_cost() | ||
| } | ||
|
|
||
| #[inline] | ||
| pub(crate) fn append_slice(&mut self, bytes: &[u8]) { | ||
| bun_core::handle_oom(self.0.write(bytes)); | ||
| } | ||
|
|
||
| #[inline] | ||
| pub(crate) fn clear_and_free(&mut self) { | ||
| self.0 = StreamBuffer::default(); | ||
| } | ||
|
|
||
| /// `n` can exceed `len()`: one writev sends the queue and the chunk behind it. | ||
| #[inline] | ||
| pub(crate) fn consume(&mut self, n: usize) { | ||
| if n >= self.len() { | ||
| self.clear_and_free(); | ||
| } else { | ||
| self.0.wrote(n); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// The byte at `position` of the stream that `replay_probe` appends. | ||
| fn probe_byte(position: usize) -> u8 { | ||
| (position ^ (position >> 8) ^ (position >> 16)) as u8 | ||
| } | ||
|
|
||
| /// `bun:internal-for-testing`: replays appends (`n >= 0`) and drains (`n < 0`) on a fresh queue. | ||
| pub(crate) fn replay_probe(global: &JSGlobalObject, frame: &CallFrame) -> JsResult<JSValue> { | ||
| let mut schedule = frame.argument(0).array_iterator(global)?; | ||
|
|
||
| let mut queue = PendingWrites::default(); | ||
| let mut appended: usize = 0; | ||
| let mut sent: usize = 0; | ||
| let mut intact = true; | ||
| let mut moves_on_drain: usize = 0; | ||
| let mut moves_on_append: usize = 0; | ||
| let mut bytes_moved: usize = 0; | ||
| let mut peak_capacity: usize = 0; | ||
|
|
||
| while let Some(entry) = schedule.next()? { | ||
| let n = entry.coerce_to_i32(global)?; | ||
| let count = n.unsigned_abs() as usize; | ||
| let unsent_before = queue.len(); | ||
| let front_before = queue.slice().as_ptr(); | ||
|
|
||
| let advanced = if n >= 0 { | ||
| let bytes: Vec<u8> = (appended..appended + count).map(probe_byte).collect(); | ||
| queue.append_slice(&bytes); | ||
| appended += bytes.len(); | ||
| 0 | ||
| } else { | ||
| let take = count.min(unsent_before); | ||
| intact &= queue.slice()[..take] | ||
| .iter() | ||
| .zip(sent..) | ||
| .all(|(byte, position)| *byte == probe_byte(position)); | ||
| sent += take; | ||
| queue.consume(count); | ||
| take | ||
| }; | ||
|
|
||
| let kept = unsent_before - advanced; | ||
| if kept > 0 && queue.slice().as_ptr() != front_before.wrapping_add(advanced) { | ||
| if n >= 0 { | ||
| moves_on_append += 1; | ||
| } else { | ||
| moves_on_drain += 1; | ||
| } | ||
| bytes_moved += kept; | ||
| } | ||
| intact &= queue.len() == appended - sent; | ||
| peak_capacity = peak_capacity.max(queue.capacity()); | ||
| } | ||
|
|
||
| let result = JSValue::create_empty_object(global, 8); | ||
| let number = |value: usize| JSValue::js_number(value as f64); | ||
| result.put(global, b"intact", JSValue::from(intact)); | ||
| result.put(global, b"sent", number(sent)); | ||
| result.put(global, b"unsent", number(queue.len())); | ||
| result.put(global, b"movesOnDrain", number(moves_on_drain)); | ||
| result.put(global, b"movesOnAppend", number(moves_on_append)); | ||
| result.put(global, b"bytesMoved", number(bytes_moved)); | ||
| result.put(global, b"capacity", number(queue.capacity())); | ||
| result.put(global, b"peakCapacity", number(peak_capacity)); | ||
| Ok(result) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| /** | ||
| * `PendingWrites` (src/runtime/socket/pending_writes.rs) holds the bytes of a | ||
| * node:net write that a send did not take. It has no JS surface of its own, so | ||
| * `pendingWritesReplayProbe` replays a schedule on a fresh queue: an entry | ||
| * `n >= 0` appends `n` bytes, an entry `n < 0` sends up to `-n` bytes from the | ||
| * front and drains `-n`. Unsent bytes that change address count as moved. | ||
| * | ||
| * test/js/node/net/net-syscall-fault.test.ts drives the same queue through a | ||
| * socket, in builds that can clamp a send. | ||
| */ | ||
| import { pendingWritesReplayProbe as probe } from "bun:internal-for-testing"; | ||
| import { describe, expect, test } from "bun:test"; | ||
|
|
||
| const step = 4096; | ||
| const tail = 255 * step; | ||
|
|
||
| describe("the queue of unsent node:net bytes", () => { | ||
| // Moving the unsent bytes to the front after every partial send cost | ||
| // tail * tail / (2 * step) bytes: 132,648,960 for the first row. | ||
| test.each([ | ||
| [tail, step], | ||
| [2 * tail, step], | ||
| [1023 * 1024, 1024], | ||
| ])("a drain of %i bytes in %i-byte steps moves no bytes and frees the allocation", (bytes, step) => { | ||
| expect(probe([bytes, ...new Array(bytes / step).fill(-step)])).toEqual({ | ||
| intact: true, | ||
| sent: bytes, | ||
| unsent: 0, | ||
| movesOnDrain: 0, | ||
| movesOnAppend: 0, | ||
| bytesMoved: 0, | ||
| capacity: 0, | ||
| peakCapacity: bytes, | ||
| }); | ||
| }); | ||
|
|
||
| // One writev sends the queue and the chunk behind it, so the count it | ||
| // reports can be larger than the queue. | ||
| test("a drain count past the end empties the queue", () => { | ||
| expect(probe([1000, -3000, 5000, -100, -4900])).toEqual({ | ||
| intact: true, | ||
| sent: 6000, | ||
| unsent: 0, | ||
| movesOnDrain: 0, | ||
| movesOnAppend: 0, | ||
| bytesMoved: 0, | ||
| capacity: 0, | ||
| peakCapacity: 5000, | ||
| }); | ||
| expect(probe([1000, -999])).toEqual({ | ||
| intact: true, | ||
| sent: 999, | ||
| unsent: 1, | ||
| movesOnDrain: 0, | ||
| movesOnAppend: 0, | ||
| bytesMoved: 0, | ||
| capacity: 1000, | ||
| peakCapacity: 1000, | ||
| }); | ||
| }); | ||
|
|
||
| // An append drops the sent prefix once it is at least as large as the unsent | ||
| // bytes. So an append moves at most the bytes sent since the move before. | ||
| test("an empty write behind a tail moves the unsent bytes once per halving", () => { | ||
| const schedule = [tail]; | ||
| for (let i = 0; i < tail / step; i++) schedule.push(0, -step); | ||
| expect(probe(schedule)).toEqual({ | ||
| intact: true, | ||
| sent: tail, | ||
| unsent: 0, | ||
| movesOnDrain: 0, | ||
| movesOnAppend: 7, | ||
| bytesMoved: (127 + 63 + 31 + 15 + 7 + 3 + 1) * step, | ||
| capacity: 0, | ||
| peakCapacity: tail, | ||
| }); | ||
| }); | ||
|
|
||
| test("writes behind a tail that is half sent fit in its allocation", () => { | ||
| const schedule = [tail, ...new Array(128).fill(-step)]; | ||
| for (let i = 0; i < 400; i++) schedule.push(1024, -step); | ||
| expect(probe(schedule)).toEqual({ | ||
| intact: true, | ||
| sent: tail + 400 * 1024, | ||
| unsent: 0, | ||
| movesOnDrain: 0, | ||
| movesOnAppend: 10, | ||
| bytesMoved: (508 + 289 + 163 + 91 + 52 + 28 + 16 + 7 + 4 + 1) * 1024, | ||
| capacity: 0, | ||
| peakCapacity: tail, | ||
| }); | ||
| }); | ||
|
|
||
| // Whether the bytes move when the allocation grows is the allocator's choice. | ||
| test("a write behind a tail that is less than half sent doubles the allocation", () => { | ||
| const { movesOnAppend, bytesMoved, ...result } = probe([tail, ...new Array(127).fill(-step), 1024]); | ||
| expect(result).toEqual({ | ||
| intact: true, | ||
| sent: 127 * step, | ||
| unsent: 128 * step + 1024, | ||
| movesOnDrain: 0, | ||
| capacity: 2 * tail, | ||
| peakCapacity: 2 * tail, | ||
| }); | ||
| expect([0, 128 * step]).toContain(bytesMoved); | ||
| expect(movesOnAppend).toBe(bytesMoved === 0 ? 0 : 1); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.