Docs don't explain how to choose between `bufread`/`read`/`write`

[Zalathar]

The encoders and decoders in this crate are available in three variants: bufread, read, and write. This makes it necessary for users to choose a variant before they can compress or decompress anything.

However, the crate documentation gives almost no indication of what the practical differences are, or why someone would choose one over the others.

• Is one of them a sharp-edged performance hazard?
• Is one of them a “safe” default choice, at the risk of being sub-optimal?
• Given that encoding/decoding is orthogonal to reading/writing, what is the intended use case for each combination?
• What's best for the simple cases of decoding an entire file into memory, or encoding a memory buffer into a file?
  • See also Decompress file example / instructions #201

[Byron]

Thanks a lot for bringing this up!

Even though I never tried to read the API documentation, I share the feeling of being somewhat confused by all these variants. It is my feeling that just a little bit of clarification would be enough to know how to think about these, with examples to hammer it home.

Is this a label you see?

Maybe it's possible to figure this out with LLM help and then contribute the documentation changes you would have wanted before writing this issue?

Otherwise, it might end up like #201 - this codebase is entirely community maintained.
Thanks for your consideration.

[Zalathar]

Maybe it's possible to figure this out with LLM help and then contribute the documentation changes you would have wanted before writing this issue?

I was already planning to dig in and contribute an answer myself, but to be honest seeing a response like this makes me feel a lot less happy about doing so. 😬

[Zalathar]

From a quick look at the code, it seems that the bufread implementations are the “real” reader-based implementations, and the read variants mostly just wrap their input stream in a BufReader and then delegate to the bufread implementation.

[Zalathar]

Ah, b91748a also seems relevant.

It looks like the read types were originally the “real” ones, but they always added an unconditional layer of buffering in front of the underlying stream.

That change introduced variants that rely on the underlying stream already being buffered, and changed the read variants to be wrappers that explicitly add a buffering layer.

It's a bit strange to me that that commit introduces a bespoke BufReader type (which still exists today), instead of just using std::io::BufReader, which would have already existed in stable at that time.

[Byron]

Maybe it's possible to figure this out with LLM help and then contribute the documentation changes you would have wanted before writing this issue?

I was already planning to dig in and contribute an answer myself, but to be honest seeing a response like this makes me feel a lot less happy about doing so. 😬

😁I can see why you would think so after reading the quote, but can happily proclaim that it wasn't meant like it may have 'sounded'. That is, I didn't mean to say that you should have created a PR rather than creating this issue, and talk to the machine to do it. If you remove the before writing this issue part, I think it fits the intent.

It's a bit strange to me that that commit introduces a bespoke BufReader type (which still exists today), instead of just using std::io::BufReader, which would have already existed in stable at that time.

Now that I read this, I am pretty sure that someone knowledgable already elaborated on this somewhere in an issue here. At that time I thought I got it, but apparently it still didn't sink in enough to be able to confidently explain it myself.
With that said, I am sure there is a good reason for not using std::io::BufReader, and maybe it's something that can still be surfaced with some detective work.

In any case, I will be looking forward to what's coming out of this - even comments here help to eventually get there, better docs, I mean.

[Zalathar]

Now that I read this, I am pretty sure that someone knowledgable already elaborated on this somewhere in an issue here. At that time I thought I got it, but apparently it still didn't sink in enough to be able to confidently explain it myself. With that said, I am sure there is a good reason for not using std::io::BufReader, and maybe it's something that can still be surfaced with some detective work.

I wasn't able to find the issue, but I think I have a pretty good guess of my own.

The in-tree buffered reader has a BufReader::with_buf constructor that accepts a Vec<u8> to use as the buffer. That constructor is needed by various ...with_buf constructors in the public flate2 API, but there's no equivalent constructor in std::io::BufReader.

[Zalathar]

	Encoder	Decoder
bufread	Requires an underlying buffered input-stream of uncompressed bytes (impl BufRead) Does not add a layer of buffering; the underlying stream's buffer is used instead Reading from the encoder yields compressed bytes	Requires an underlying buffered input-stream of compressed bytes (impl BufRead) Does not add a layer of buffering; the underlying stream's buffer is used instead Reading from the decoder yields uncompressed bytes
read	Requires an underlying input-stream of uncompressed bytes (impl Read) Adds an intermediate 32 KB input buffer Reading from the encoder yields compressed bytes	Requires an underlying input-stream of compressed bytes (impl Read) Adds an intermediate 32 KB input buffer Reading from the decoder yields uncompressed bytes
write	Requires an underlying output-stream that wants compressed bytes(impl Write) Adds an intermediate 32 KB output buffer Writing uncompressed bytes to the encoder causes it to write compressed bytes to the underlying output-stream	Requires an underlying output-stream that wants uncompressed bytes (impl Write) Adds an intermediate 32 KB output buffer Writing compressed bytes to the decoder causes it to write uncompressed bytes to the underlying output-stream

[jongiddy]

For sync code prefer to use bufread encoders/decoders. If you have a Read type then the read encoders/decoders are just a convenience to avoid having to wrap the Read in a BufReader yourself. Apart from backwards-compatibility concerns, we could easily get rid of the read types.

The write encoders/decoders are most useful when there is no way to create a BufRead type, notably async handlers such as hyper's body frames (consider adding decoding in place of to_ascii_uppercase in the example).

[Byron]

Thanks @jongiddy for chiming in. I am pretty sure you were that person whose comment I vaguely remember :).

Do you want to open a PR for the table, maybe even with @jongiddy's comment incorporated? But any kind of commit with your name would be appreciated for attribution, and I could make the final edits from there, with @jongiddy as reviewer maybe.
WDYT?

[Zalathar]

To make life easier for users, I wonder if it's better to only mention bufread and write in the main explanation/table.

Then read would only be mentioned in a relatively-minor note, as something that exists for backwards-compatibility with code that predates bufread.

“Always prefer bufread, with an intermediate std::io::BufReader if the compiler complains about BufRead” seems a lot simpler than actually trying to explain the read variants.

[fintelia]

Yeah, the read variants are just the bufread ones wrapped in a BufReader. The only nuance is that this crate has its own BufReader implementation (because the standard library's version doesn't have a BufReader::with_buf method) and the default buffer size for them is 32 KB (IIRC the standard library defaults to 8KB).

[Byron]

All that sounds good to me. To me it would be easier to discuss if there was a proposed change via PR though, so everything is in context.