propose adding BufferedTee to the std.io #19032
Conversation
BufferedTee provides reader interface to the consumer. Data read by consumer
is also written to the output. Output is hold lookahead_size bytes behind
consumer. Allowing consumer to put back some bytes to be read again. On flush
all consumed bytes are flushed to the output.
input -> tee -> consumer
|
output
input - underlying unbuffered reader
output - writer, receives data read by consumer
consumer - uses provided reader interface
If lookahead_size is zero output always has same bytes as consumer.
squeek502
changed the title
|
rather than |
In this case we have readers in the chain: checksum and decompressor. If the decompressor overshoots whether by using read or peek checksum will be wrong. Peek will also pull data through checksum. |
There was a problem hiding this comment.
I think this is very nice, thanks again for proposing it. Would you consider adding the git.zig changes you made in #18967 (comment) to this PR as a separate commit? Having those changes also included would give a concrete example of where this is useful.
lib/std/io/buffered_tee.zig
Outdated
| //! BufferedTee provides reader interface to the consumer. Data read by consumer | ||
| //! is also written to the output. Output is hold lookahead_size bytes behind | ||
| //! consumer. Allowing consumer to put back some bytes to be read again. On flush | ||
| //! all consumed bytes are flushed to the output. | ||
| //! | ||
| //! input -> tee -> consumer | ||
| //! | | ||
| //! output | ||
| //! | ||
| //! input - underlying unbuffered reader | ||
| //! output - writer, receives data read by consumer | ||
| //! consumer - uses provided reader interface | ||
| //! | ||
| //! If lookahead_size is zero output always has same bytes as consumer. | ||
| //! |
There was a problem hiding this comment.
The namespace of this file isn't accessible to users since BufferedTee and bufferedTee are exposed directly under std.io via pub const BufferedTee = @import("io/buffered_tee.zig").BufferedTee; in io.zig, so this doc comment won't show up in the generated documentation. If you move this right above pub fn BufferedTee and change it to a regular doc comment (///), then it'll show up as the documentation for that type.
lib/std/io/buffered_tee.zig
Outdated
| return BufferedTee( | ||
| buffer_size, | ||
| lookahead_size, | ||
| @TypeOf(input), | ||
| @TypeOf(output), | ||
| ){ |
There was a problem hiding this comment.
| return BufferedTee( | |
| buffer_size, | |
| lookahead_size, | |
| @TypeOf(input), | |
| @TypeOf(output), | |
| ){ | |
| return .{ |
Just makes it a little more concise, since the full type has already been written out as the return type.
There was a problem hiding this comment.
This is ready to land when @ianic and @ianprime0509 consider it ready
Is it possible to import If not I suggest to merge this and I'll make follow up PR for changes in git.zig. |
Yes, that's the same as |
src/Package/Fetch/git.zig
Outdated
| } | ||
|
|
||
| /// Performs the first pass over the packfile data for index construction. | ||
| // Performs the first pass over the packfile data for index construction. |
There was a problem hiding this comment.
| // Performs the first pass over the packfile data for index construction. | |
| /// Performs the first pass over the packfile data for index construction. |
src/Package/Fetch/git.zig
Outdated
|
|
||
| switch (entry_header) { | ||
| .commit, .tree, .blob, .tag => |object| { | ||
| inline .commit, .tree, .blob, .tag => |object, tag| { |
There was a problem hiding this comment.
This switch arm doesn't need to be inline, as long as @tagName(tag) below is replaced with @tagName(entry_header) (this was a minor thing I fixed in my workaround PR which wasn't directly related to the workaround).
Introduced in ziglang#19032 as a fix for ziglang#18967. Not needed any more after ziglang#19253.
While analyzing #18967 I came to the idea of this kind of stream, Ian named it BufferedTee. The main idea is to allow consumer to put back to the stream some bytes it has already read, allowing it to lookahead few bytes and letter change it's mind and not consume them. Other consumers of the same stream should not be affected by this. BufferedTee is holding other consumers some number of bytes behind 'main' consumer allowing it to put back that number of bytes.
I feel that except helping with this particular case there can be other cases using lookahead approach which can found this type of stream useful. That is the reason behind this PR.