Skip to content
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

fix(fs): WriteStream pending write fastpath #16856

Merged
merged 41 commits into from
Jan 31, 2025
Merged
Changes from 3 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
6ef67d3
dont write out of order
dylan-conway Jan 29, 2025
dc04802
update
dylan-conway Jan 29, 2025
21a4816
update
dylan-conway Jan 29, 2025
ce66af7
fixup
dylan-conway Jan 29, 2025
b8ff702
one more
dylan-conway Jan 29, 2025
135c13b
auto flush
dylan-conway Jan 29, 2025
07feacb
progress
dylan-conway Jan 29, 2025
9086433
buffered stream
dylan-conway Jan 30, 2025
08e8588
missing onWrite
dylan-conway Jan 30, 2025
e80764b
Merge branch 'main' into dylan/fix-pending-write
dylan-conway Jan 30, 2025
991871a
fix windows zig build
dylan-conway Jan 30, 2025
3ff9aa2
update ref
dylan-conway Jan 30, 2025
1242201
clear capacity
dylan-conway Jan 30, 2025
a21c44c
test
dylan-conway Jan 30, 2025
5c13864
fix windows build
dylan-conway Jan 30, 2025
7dce37a
Update src/bun.js/webcore/streams.zig
dylan-conway Jan 30, 2025
d33699b
runPending update
dylan-conway Jan 30, 2025
07b1dfb
mini eventloop
dylan-conway Jan 30, 2025
17b5d04
stderr
dylan-conway Jan 30, 2025
6c1520c
update
dylan-conway Jan 30, 2025
d34465b
Don't spin loop
Jarred-Sumner Jan 30, 2025
a707cd9
Update sys.zig
Jarred-Sumner Jan 30, 2025
3aa413c
Update streams.zig
Jarred-Sumner Jan 30, 2025
4040d9e
fix
Jarred-Sumner Jan 30, 2025
11e477c
Update sys.zig
Jarred-Sumner Jan 30, 2025
3d2bdcf
Update sys.zig
Jarred-Sumner Jan 30, 2025
ee815bd
Match node's behavior with force sync
Jarred-Sumner Jan 30, 2025
0f410dd
Update streams.zig
Jarred-Sumner Jan 30, 2025
d561ea3
Ensure we actually set blocking
Jarred-Sumner Jan 30, 2025
1a6181b
Workaround for zig std lib decision
Jarred-Sumner Jan 30, 2025
b397a20
Update PipeWriter.zig
Jarred-Sumner Jan 30, 2025
0f36adb
Delete the function that is labeled as not being correct
Jarred-Sumner Jan 30, 2025
4309c75
Update tty.ts
Jarred-Sumner Jan 30, 2025
c87cb34
update
dylan-conway Jan 30, 2025
5401b5f
update
dylan-conway Jan 31, 2025
f9d5d91
write fast
dylan-conway Jan 31, 2025
6bef911
oops
dylan-conway Jan 31, 2025
5690100
update
dylan-conway Jan 31, 2025
f6b96ce
Merge branch 'main' into dylan/fix-pending-write
dylan-conway Jan 31, 2025
fe2ae42
block
dylan-conway Jan 31, 2025
2de9cc2
fix it!!
dylan-conway Jan 31, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 43 additions & 16 deletions src/io/PipeWriter.zig
Original file line number Diff line number Diff line change
Expand Up @@ -41,35 +41,35 @@ pub fn PosixPipeWriter(
};
}

fn _tryWriteWithWriteFn(this: *This, buf_: []const u8, comptime write_fn: *const fn (bun.FileDescriptor, []const u8) JSC.Maybe(usize)) WriteResult {
fn _tryWriteWithWriteFn(this: *This, buf: []const u8, comptime write_fn: *const fn (bun.FileDescriptor, []const u8) JSC.Maybe(usize)) WriteResult {
const fd = getFd(this);
var buf = buf_;
var rest = buf;

while (buf.len > 0) {
switch (write_fn(fd, buf)) {
while (rest.len > 0) {
switch (write_fn(fd, rest)) {
.err => |err| {
if (err.isRetry()) {
return .{ .pending = buf_.len - buf.len };
return .{ .pending = buf.len - rest.len };
}

if (err.getErrno() == .PIPE) {
return .{ .done = buf_.len - buf.len };
return .{ .done = buf.len - rest.len };
}

return .{ .err = err };
},

.result => |wrote| {
if (wrote == 0) {
return .{ .done = buf_.len - buf.len };
return .{ .done = buf.len - rest.len };
}

buf = buf[wrote..];
rest = rest[wrote..];
},
}
}

return .{ .wrote = buf_.len - buf.len };
return .{ .wrote = buf.len - rest.len };
}

fn writeToFileType(comptime file_type: FileType) *const (fn (bun.FileDescriptor, []const u8) JSC.Maybe(usize)) {
Expand Down Expand Up @@ -394,8 +394,8 @@ pub fn PosixStreamingWriter(
is_done: bool = false,
closed_without_reporting: bool = false,

// TODO:
chunk_size: usize = 0,
// TODO: configurable?
const chunk_size: usize = 1024 * 4;
Copy link
Member

Choose a reason for hiding this comment

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

should be configurable, though it doesn't have to block this PR.

node:fs streams have this option. we can add an options argument into Bun.file().writer()

https://nodejs.org/api/fs.html#filehandlecreatewritestreamoptions


pub fn memoryCost(this: *const @This()) usize {
return @sizeOf(@This()) + this.buffer.capacity;
Expand Down Expand Up @@ -586,24 +586,51 @@ pub fn PosixStreamingWriter(
return .{ .done = 0 };
}

if (this.buffer.items.len + buf.len < this.chunk_size) {
if (this.buffer.items.len > 0 or buf.len < chunk_size) {
this.buffer.appendSlice(buf) catch {
return .{ .err = bun.sys.Error.oom };
};
return .{ .pending = buf.len };
}

return .{ .pending = 0 };
this.head = 0;

if (this.buffer.items.len > 0) {
this.buffer.appendSlice(buf) catch {
return .{ .err = bun.sys.Error.oom };
};
const rc = @This()._tryWrite(this, this.buffer.items);
switch (rc) {
.pending => |amt| {
onWrite(this.parent, amt, .pending);
registerPoll(this);
},
.wrote => |amt| {
if (amt < this.buffer.items.len) {
onWrite(this.parent, amt, .pending);
} else {
this.buffer.clearRetainingCapacity();
onWrite(this.parent, amt, .drained);
}
},
.done => |amt| {
this.buffer.clearRetainingCapacity();
onWrite(this.parent, amt, .end_of_file);
return .{ .done = amt };
},
else => {},
}
return rc;
}

const rc = @This()._tryWrite(this, buf);
this.head = 0;

switch (rc) {
.pending => |amt| {
this.buffer.appendSlice(buf[amt..]) catch {
return .{ .err = bun.sys.Error.oom };
};

onWrite(this.parent, amt, .pending);

registerPoll(this);
},
.wrote => |amt| {
Expand Down