diff --git a/src/terminal/c/formatter.zig b/src/terminal/c/formatter.zig index ded71a68d..aad33a14e 100644 --- a/src/terminal/c/formatter.zig +++ b/src/terminal/c/formatter.zig @@ -169,16 +169,22 @@ pub fn format( const wrapper = formatter_ orelse return .invalid_value; if (!writer.valid()) return .invalid_value; - var adapter: io_c.WriterAdapter = .init(writer); - switch (wrapper.kind) { - .terminal => |*t| t.format(adapter.writer()) catch { - if (adapter.invalid_write) return .limit_exceeded; - if (adapter.callback_failed) return .io_error; - return .io_error; - }, + // Batch the formatter's many small writes into few callback calls. The + // trailing flush upholds the API contract that success means the + // callback has accepted every byte. + var buffer: [io_c.WriterAdapter.recommended_buffer_len]u8 = undefined; + var adapter: io_c.WriterAdapter = .initBuffered(writer, &buffer); + format_: { + switch (wrapper.kind) { + .terminal => |*t| t.format(adapter.writer()) catch break :format_, + } + adapter.writer().flush() catch break :format_; + return .success; } - return .success; + if (adapter.invalid_write) return .limit_exceeded; + if (adapter.callback_failed) return .io_error; + return .io_error; } pub fn format_buf( diff --git a/src/terminal/c/io.zig b/src/terminal/c/io.zig index 014935dc0..30a431b6f 100644 --- a/src/terminal/c/io.zig +++ b/src/terminal/c/io.zig @@ -228,10 +228,14 @@ pub const ReaderAdapter = struct { } }; -/// Adapts a `GhosttyWriter` to an unbuffered `std.Io.Writer`. +/// Adapts a `GhosttyWriter` to a `std.Io.Writer`. /// -/// Keeping the Zig writer unbuffered ensures success means the C callback has -/// already accepted every byte; there is no hidden flush step at API return. +/// The adapter may be given a buffer via `initBuffered` so that the many +/// small writes typical of streaming producers (e.g. the formatter) are +/// batched into far fewer C callback invocations. A buffered adapter's owner +/// must call `flush` on the interface before reporting success so that the +/// public contract holds: success means the C callback has already accepted +/// every byte. pub const WriterAdapter = struct { /// Copy of the C callback pair. The pointed-to userdata remains borrowed. destination: Writer, @@ -239,7 +243,8 @@ pub const WriterAdapter = struct { /// Zig-facing interface whose drain vtable points back to this adapter. interface: std.Io.Writer, - /// Bytes accepted by successful callback invocations. + /// Bytes accepted by successful callback invocations. Buffered bytes are + /// counted only once a drain or flush hands them to the callback. offset: usize = 0, /// The callback returned false. @@ -248,13 +253,27 @@ pub const WriterAdapter = struct { /// The callback was NULL or offset accounting overflowed. invalid_write: bool = false, + /// The buffer length used by the C API entry points that stream through + /// a callback. 4 KiB amortizes callback overhead while keeping every + /// adapter's stack footprint modest, mirroring ReaderAdapter's + /// transfer_buffer sizing. + pub const recommended_buffer_len = 4096; + + /// An unbuffered adapter: every Zig write reaches the callback + /// immediately and no flush is required before returning. pub fn init(destination: Writer) WriterAdapter { + return initBuffered(destination, &.{}); + } + + /// A buffered adapter. The buffer must outlive the adapter and remain + /// at a stable address (it is referenced, not copied). The owner must + /// flush the interface before treating the operation as successful. + pub fn initBuffered(destination: Writer, buffer: []u8) WriterAdapter { return .{ .destination = destination, .interface = .{ - // With no buffer, every Zig write reaches `drain` immediately. .vtable = &.{ .drain = drain }, - .buffer = &.{}, + .buffer = buffer, .end = 0, }, }; @@ -293,19 +312,49 @@ pub const WriterAdapter = struct { }; } - /// Implement the sole primitive required by an unbuffered std.Io.Writer. + /// Write one slice, batching through the interface buffer when one is + /// attached. Slices that fit are parked in the buffer (a later drain or + /// flush hands them to the callback); anything larger goes to the + /// callback directly after the buffer is emptied to preserve ordering. + fn writeSlice( + self: *WriterAdapter, + writer_: *std.Io.Writer, + slice: []const u8, + ) std.Io.Writer.Error!void { + const buffer = writer_.buffer; + if (slice.len <= buffer.len - writer_.end) { + @memcpy(buffer[writer_.end..][0..slice.len], slice); + writer_.end += slice.len; + return; + } + + if (writer_.end > 0) { + try self.writeAll(buffer[0..writer_.end]); + writer_.end = 0; + } + + if (slice.len <= buffer.len) { + @memcpy(buffer[0..slice.len], slice); + writer_.end = slice.len; + } else { + try self.writeAll(slice); + } + } + + /// Implement the sole primitive required by a std.Io.Writer. /// /// Zig represents a vector write as ordinary slices followed by the last - /// slice repeated `splat` times. The C callback has no vector form, so this - /// method expands that representation into ordered all-or-nothing calls - /// and returns the total logical byte count consumed. + /// slice repeated `splat` times. The C callback has no vector form, so + /// this method expands that representation, batching through the + /// interface buffer when one is attached, and returns the total logical + /// byte count consumed from `data`. Bytes parked in the buffer count as + /// consumed; `std.Io.Writer.flush` drains them via this same method. fn drain( writer_: *std.Io.Writer, data: []const []const u8, splat: usize, ) std.Io.Writer.Error!usize { assert(data.len > 0); // The final element is always the splat pattern. - assert(writer_.end == 0); // This adapter intentionally has no buffer. // The vtable receives the embedded Writer rather than our adapter. const self: *WriterAdapter = @alignCast(@fieldParentPtr( @@ -313,10 +362,16 @@ pub const WriterAdapter = struct { writer_, )); + // Buffered bytes must reach the destination before any of `data`. + if (writer_.end > 0) { + try self.writeAll(writer_.buffer[0..writer_.end]); + writer_.end = 0; + } + var consumed: usize = 0; // Every element except the last is written exactly once. for (data[0 .. data.len - 1]) |slice| { - try self.writeAll(slice); + try self.writeSlice(writer_, slice); consumed = std.math.add(usize, consumed, slice.len) catch { self.invalid_write = true; return error.WriteFailed; @@ -326,8 +381,28 @@ pub const WriterAdapter = struct { // std.Io uses the final element as a compact repeated pattern. A // splat of zero means the final element is not part of this write. const pattern = data[data.len - 1]; - for (0..splat) |_| { - try self.writeAll(pattern); + if (pattern.len == 1 and writer_.buffer.len > 0) { + // Single-byte splats (e.g. runs of blank cells) are memset + // into the buffer in bulk rather than repeated one write at + // a time. + const buffer = writer_.buffer; + var remaining = splat; + while (remaining > 0) { + if (writer_.end == buffer.len) { + try self.writeAll(buffer); + writer_.end = 0; + } + const n = @min(remaining, buffer.len - writer_.end); + @memset(buffer[writer_.end..][0..n], pattern[0]); + writer_.end += n; + remaining -= n; + } + consumed = std.math.add(usize, consumed, splat) catch { + self.invalid_write = true; + return error.WriteFailed; + }; + } else for (0..splat) |_| { + try self.writeSlice(writer_, pattern); consumed = std.math.add(usize, consumed, pattern.len) catch { self.invalid_write = true; return error.WriteFailed; @@ -502,6 +577,92 @@ test "WriterAdapter writes vectors and splats" { try std.testing.expect(!adapter.callback_failed); } +test "WriterAdapter buffered batches small writes" { + const Context = struct { + data: [256]u8 = undefined, + len: usize = 0, + calls: usize = 0, + + fn write( + userdata: ?*anyopaque, + data: [*]const u8, + len: usize, + ) callconv(lib.calling_conv) bool { + const self: *@This() = @ptrCast(@alignCast(userdata.?)); + @memcpy(self.data[self.len..][0..len], data[0..len]); + self.len += len; + self.calls += 1; + return true; + } + }; + + var context: Context = .{}; + var buffer: [64]u8 = undefined; + var adapter: WriterAdapter = .initBuffered(.{ + .write = &Context.write, + .userdata = &context, + }, &buffer); + + // Many small writes fit in the buffer: no callback yet. + for (0..16) |_| try adapter.interface.writeAll("ab"); + try std.testing.expectEqual(@as(usize, 0), context.calls); + + // Flush delivers everything in a single call. + try adapter.interface.flush(); + try std.testing.expectEqual(@as(usize, 1), context.calls); + try std.testing.expectEqualStrings( + "ab" ** 16, + context.data[0..context.len], + ); + try std.testing.expectEqual(@as(usize, 32), adapter.offset); +} + +test "WriterAdapter buffered splats and large writes" { + const Context = struct { + data: [256]u8 = undefined, + len: usize = 0, + calls: usize = 0, + + fn write( + userdata: ?*anyopaque, + data: [*]const u8, + len: usize, + ) callconv(lib.calling_conv) bool { + const self: *@This() = @ptrCast(@alignCast(userdata.?)); + @memcpy(self.data[self.len..][0..len], data[0..len]); + self.len += len; + self.calls += 1; + return true; + } + }; + + var context: Context = .{}; + var buffer: [16]u8 = undefined; + var adapter: WriterAdapter = .initBuffered(.{ + .write = &Context.write, + .userdata = &context, + }, &buffer); + + // A splat larger than the buffer is chunked, not one call per byte. + try adapter.interface.splatByteAll(' ', 40); + // A write larger than the buffer flushes and goes to the callback + // directly. + try adapter.interface.writeAll("0123456789abcdef0"); // 17 > 16 + try adapter.interface.writeAll("xy"); + try adapter.interface.flush(); + + try std.testing.expectEqualStrings( + " " ** 40 ++ "0123456789abcdef0" ++ "xy", + context.data[0..context.len], + ); + try std.testing.expectEqual( + @as(usize, context.len), + adapter.offset, + ); + // Chunked delivery: far fewer calls than bytes. + try std.testing.expect(context.calls <= 6); +} + test "WriterAdapter makes callback failure sticky" { const Context = struct { calls: usize = 0, diff --git a/src/terminal/c/snapshot.zig b/src/terminal/c/snapshot.zig index 45ac8a1e3..e978ca49b 100644 --- a/src/terminal/c/snapshot.zig +++ b/src/terminal/c/snapshot.zig @@ -542,14 +542,18 @@ pub fn encode( if (continuation.result != .success) return continuation.result; defer native.gpa().free(continuation.bytes); - // Stream the snapshot directly through the callback adapter. - var adapter: io_c.WriterAdapter = .init(writer); + // Stream the snapshot through the callback adapter, batching the + // encoder's writes into few callback invocations. + var buffer: [io_c.WriterAdapter.recommended_buffer_len]u8 = undefined; + var adapter: io_c.WriterAdapter = .initBuffered(writer, &buffer); snapshot_core.encode( native.gpa(), &adapter.interface, native, .{ .continuation = continuationValue(continuation.bytes) }, ) catch |err| return mapEncodeError(err, &adapter); + adapter.interface.flush() catch + return mapEncodeError(error.WriteFailed, &adapter); return .success; } diff --git a/src/terminal/c/terminal.zig b/src/terminal/c/terminal.zig index 55d488158..8732c5dc8 100644 --- a/src/terminal/c/terminal.zig +++ b/src/terminal/c/terminal.zig @@ -810,15 +810,20 @@ pub fn continuation_write( // The callback was validated above, so invalid_write can only mean output // accounting overflow. Keep that distinct from callback rejection. - var adapter: c_io.WriterAdapter = .init(writer); - continuationWriteIo(terminal_, &adapter.interface) catch |err| { - if (err == error.WriteFailed) { - if (adapter.invalid_write) return .limit_exceeded; - if (adapter.callback_failed) return .io_error; - } - return continuationErrorResult(err); - }; - return .success; + var buffer: [c_io.WriterAdapter.recommended_buffer_len]u8 = undefined; + var adapter: c_io.WriterAdapter = .initBuffered(writer, &buffer); + write: { + continuationWriteIo(terminal_, &adapter.interface) catch |err| switch (err) { + error.WriteFailed => break :write, + else => return continuationErrorResult(err), + }; + adapter.interface.flush() catch break :write; + return .success; + } + + if (adapter.invalid_write) return .limit_exceeded; + if (adapter.callback_failed) return .io_error; + return continuationErrorResult(error.WriteFailed); } pub fn continuation_buf(