From b19d0fb0fd3e4ff023968e77f01848ffb9dce360 Mon Sep 17 00:00:00 2001 From: Lucas Santos <117400842+LucasSantos91@users.noreply.github.com> Date: Mon, 23 Sep 2024 21:20:27 -0300 Subject: [PATCH] Improve efficiency of buffered_reader. (#21256) The previous implementation of buffered_reader always reads from the unbuffered reader into the internal buffer, and then dumps the data onto the destination. This is inefficient, as sometimes it's possible to read directly into the destination. The previous strategy generates more memory copies and unbuffered reads than necessary. --- lib/std/io/buffered_reader.zig | 72 ++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/lib/std/io/buffered_reader.zig b/lib/std/io/buffered_reader.zig index ca132202a7..bcf54fb882 100644 --- a/lib/std/io/buffered_reader.zig +++ b/lib/std/io/buffered_reader.zig @@ -17,26 +17,27 @@ pub fn BufferedReader(comptime buffer_size: usize, comptime ReaderType: type) ty const Self = @This(); pub fn read(self: *Self, dest: []u8) Error!usize { - var dest_index: usize = 0; - - while (dest_index < dest.len) { - const written = @min(dest.len - dest_index, self.end - self.start); - @memcpy(dest[dest_index..][0..written], self.buf[self.start..][0..written]); - if (written == 0) { - // buf empty, fill it - const n = try self.unbuffered_reader.read(self.buf[0..]); - if (n == 0) { - // reading from the unbuffered stream returned nothing - // so we have nothing left to read. - return dest_index; - } - self.start = 0; - self.end = n; - } - self.start += written; - dest_index += written; + // First try reading from the already buffered data onto the destination. + const current = self.buf[self.start..self.end]; + if (current.len != 0) { + const to_transfer = @min(current.len, dest.len); + @memcpy(dest[0..to_transfer], current[0..to_transfer]); + self.start += to_transfer; + return to_transfer; } - return dest.len; + + // If dest is large, read from the unbuffered reader directly into the destination. + if (dest.len >= buffer_size) { + return self.unbuffered_reader.read(dest); + } + + // If dest is small, read from the unbuffered reader into our own internal buffer, + // and then transfer to destination. + self.end = try self.unbuffered_reader.read(&self.buf); + const to_transfer = @min(self.end, dest.len); + @memcpy(dest[0..to_transfer], self.buf[0..to_transfer]); + self.start = to_transfer; + return to_transfer; } pub fn reader(self: *Self) Reader { @@ -134,12 +135,13 @@ test "Block" { var test_buf_reader: BufferedReader(4, BlockReader) = .{ .unbuffered_reader = BlockReader.init(block, 2), }; + const reader = test_buf_reader.reader(); var out_buf: [4]u8 = undefined; - _ = try test_buf_reader.read(&out_buf); + _ = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, &out_buf, block); - _ = try test_buf_reader.read(&out_buf); + _ = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, &out_buf, block); - try testing.expectEqual(try test_buf_reader.read(&out_buf), 0); + try testing.expectEqual(try reader.readAll(&out_buf), 0); } // len out < block @@ -147,14 +149,15 @@ test "Block" { var test_buf_reader: BufferedReader(4, BlockReader) = .{ .unbuffered_reader = BlockReader.init(block, 2), }; + const reader = test_buf_reader.reader(); var out_buf: [3]u8 = undefined; - _ = try test_buf_reader.read(&out_buf); + _ = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, &out_buf, "012"); - _ = try test_buf_reader.read(&out_buf); + _ = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, &out_buf, "301"); - const n = try test_buf_reader.read(&out_buf); + const n = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, out_buf[0..n], "23"); - try testing.expectEqual(try test_buf_reader.read(&out_buf), 0); + try testing.expectEqual(try reader.readAll(&out_buf), 0); } // len out > block @@ -162,12 +165,13 @@ test "Block" { var test_buf_reader: BufferedReader(4, BlockReader) = .{ .unbuffered_reader = BlockReader.init(block, 2), }; + const reader = test_buf_reader.reader(); var out_buf: [5]u8 = undefined; - _ = try test_buf_reader.read(&out_buf); + _ = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, &out_buf, "01230"); - const n = try test_buf_reader.read(&out_buf); + const n = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, out_buf[0..n], "123"); - try testing.expectEqual(try test_buf_reader.read(&out_buf), 0); + try testing.expectEqual(try reader.readAll(&out_buf), 0); } // len out == 0 @@ -175,8 +179,9 @@ test "Block" { var test_buf_reader: BufferedReader(4, BlockReader) = .{ .unbuffered_reader = BlockReader.init(block, 2), }; + const reader = test_buf_reader.reader(); var out_buf: [0]u8 = undefined; - _ = try test_buf_reader.read(&out_buf); + _ = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, &out_buf, ""); } @@ -185,11 +190,12 @@ test "Block" { var test_buf_reader: BufferedReader(5, BlockReader) = .{ .unbuffered_reader = BlockReader.init(block, 2), }; + const reader = test_buf_reader.reader(); var out_buf: [4]u8 = undefined; - _ = try test_buf_reader.read(&out_buf); + _ = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, &out_buf, block); - _ = try test_buf_reader.read(&out_buf); + _ = try reader.readAll(&out_buf); try testing.expectEqualSlices(u8, &out_buf, block); - try testing.expectEqual(try test_buf_reader.read(&out_buf), 0); + try testing.expectEqual(try reader.readAll(&out_buf), 0); } }