Tracking Issue for bufreader_peek

Feature gate: #![feature(bufreader_peek)]

This is a tracking issue for the BufReader::peek method.

Public API

// std::io

impl BufReader {
    fn peek(&mut self, n: usize) -> io::Result<&[u8]>
}

Steps / History

ACP Accepted: ACP: BufReader::peek libs-team#417
Implementation: implement BufReader::peek #128406
Final comment period (FCP)¹
Stabilization PR

Unresolved Questions

what should happen if n > capacity? options are: short slice, error, or panic.
should the buffer be completely filled, or should it only read the number of bytes requested?

https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html ↩

@rustbot

@rustbot claim

starting work on implementation

Rollup merge of rust-lang#128406 - lolbinarycat:bufreader_peek, r=Mar…

Unrolled build for rust-lang#128406

Rollup merge of #128406 - lolbinarycat:bufreader_peek, r=Mark-Simulacrum

calling peek should allow you to call consume with the same or a smaller n value. currently consume is only guaranteed to be well-behaved when calling it after fill_buf.

Rollup merge of rust-lang#129675 - lolbinarycat:bufreader_peek_unsize…

Sorry if this is the wrong place to ask, but have alternative names been considered?

Currently, the fill_buf method ensures that the internal buffer contains at least one byte, but notably doesn't guarantee that the buffer ends up full.

Meanwhile peek sounds like it does even less work. But actually it is a more aggressive version of fill_buf that fills the internal buffer even if there are still some bytes left in it.

I'm not sure what the most intuitive name would be, but perhaps something like fill_buf_all or fill_buf_exact?

The difference is that fill_buf is a low-level primitive, meant as a building block for other operations, while peek is a useful function by itself.

The fact that peek technically "does more work" sometimes is irrelevant IMO, since it only does that work if it needs to in order to fulfill its semantics.

While their implementations are similar, their semantics and intended usecase are not.

Using the term "peek" for "look ahead in a stream without advancing it" is a well-established programming convention. At a quick glance i found peek functions in Go and C++ with nearly identical semantics, and I'm sure there's many more.

Rust typically follows existing naming conbentions where applicable, it's why print isn't called write_formatted.

I don't want to get too philosophical but since you can implement fill_buf by simply passing 1 to peek and then calling buffer I'd argue fill_buf is higher-level than peek.

Anyway, the name peek does make sense. But it's not the name I expected to mean "fill at least n bytes". I did find it by looking for exactly what it can do but it looked confusing and I had to click through and carefully read the doc to realize it's doing what I need.

But that can be improved by changing the short doc to say "Attempts to fill the buffer with at least n bytes".

I don't want to get too philosophical but since you can implement fill_buf by simply passing 1 to peek and then calling buffer I'd argue fill_buf is higher-level than peek.

Actually a lot of low-level functions can be implemented trivially if you already have a higher-level function. If you already have an implementation for Write::write_all, that is also a valid implementation for Write::write. The important distinction is that the lower level function provides less guarantees.

Anyway, the name peek does make sense. But it's not the name I expected to mean "fill at least n bytes". I did find it by looking for exactly what it can do but it looked confusing and I had to click through and carefully read the doc to realize it's doing what I need.

I would argue this is an X/Y problem. Filling the buffer is not itself a useful operation, but is instead the means to an end. I'm curious why you would want to fill the buffer if not to look ahead in the stream?

I would argue this is an X/Y problem. Filling the buffer is not itself a useful operation, but is instead the means to an end. I'm curious why you would want to fill the buffer if not to look ahead in the stream?

It is a question of consistent terminology. The existing fill_buf method could totally have been called peek or peek_one instead given that it already returns the underlying buffer and the prior art from other languages. But knowing that there's a method that only partially fills the buffer, it is natural to look for a similarly named method that completely fills the buffer. Either peek_one/peek or fill_buf/fill_buf_all would make sense to me. But only the latter can be adopted in a backwards compatible way

Write::write_all, that is also a valid implementation for Write::write

Only if you're writing one byte at a time which is quite less performant compared to my peek example. Anyway, you're right and a better example is one can implement subtraction as addition with some bit operations but also addition as subtraction with some bit operations.

why you would want to fill the buffer if not to look ahead in the stream?

To have at least 4 bytes of base64 to pass into a decoding function. (The decoding function can take more but taking less would confuse it.)

The existing fill_buf method could totally have been called peek or peek_one instead given that it already returns the underlying buffer and the prior art from other languages. But knowing that there's a method that only partially fills the buffer, it is natural to look for a similarly named method that completely fills the buffer. Either peek_one/peek or fill_buf/fill_buf_all would make sense to me.

Exactly. peek makes it look like the operation is very different but they are in fact very similar.

It is a question of consistent terminology. The existing fill_buf method could totally have been called peek or peek_one instead given that it already returns the underlying buffer and the prior art from other languages. But knowing that there's a method that only partially fills the buffer, it is natural to look for a similarly named method that completely fills the buffer. Either peek_one/peek or fill_buf/fill_buf_all would make sense to me. But only the latter can be adopted in a backwards compatible way

peek does not neccarily completely fill the buffer. Even if it did, that fact would be an internal implementation detail.

Write::write_all, that is also a valid implementation for Write::write

Only if you're writing one byte at a time which is quite less performant compared to my peek example. Anyway, you're right and a better example is one can implement subtraction as addition with some bit operations but also addition as subtraction with some bit operations.

What I mean is that if you have a writer that never performs partial writes, you can implement write_all and write identically. Indeed, this is how the Vec<u8> impl works.

why you would want to fill the buffer if not to look ahead in the stream?

To have at least 4 bytes of base64 to pass into a decoding function. (The decoding function can take more but taking less would confuse it.)

Why not just read 4 bytes at a time?

The existing fill_buf method could totally have been called peek or peek_one instead given that it already returns the underlying buffer and the prior art from other languages. But knowing that there's a method that only partially fills the buffer, it is natural to look for a similarly named method that completely fills the buffer. Either peek_one/peek or fill_buf/fill_buf_all would make sense to me.

Exactly. peek makes it look like the operation is very different but they are in fact very similar.

Aren't most operations on buffered streams very similar? peek could just as easily be called read_no_consume, and that would be just as correct as fill_buf_at_least. Should we also rename read to fill_buf_and_consume, since that's what it's equivalent to?

Even if fill_buf_at_least was a better name (which I don't believe it is), is it so much better that it is worth it to back through the ACP process, rename the feature, and break all code that is currently using peek?

Even if I wanted it renamed, I have no authority in that decision.

The only productive options you have at this point are to either to suggest a documentation change that clarifies this while also matching the semantics of peek, or to file an ACP.

Why not just read 4 bytes at a time?

Issues around zero-initializing the buffer
Intermediate copy even when slice is used (might be optimizable, IDK)
Cannot SIMD
If the reader is dyn then each chunk incurs the cost of dynamic dispatch

Even if fill_buf_at_least was a better name (which I don't believe it is), is it so much better that it is worth it to back through the ACP process, rename the feature, and break all code that is currently using peek?

I'm not excited about either of those but I could imagine doing so if there is any better.

ACP 569 which proposes to generalize this to a trait also proposes a bit more general API in that the peek method returns "at least n" rather than "exactly n" bytes.
It seems like peek could get that generalization too.

@the8472

@the8472 yeah, I didn't consider stuff like slices when I was writing my original proposal, eliminating that copy within generic code does seem worthwhile.

what should happen if n > capacity?

It should not return a shorter slice.

A slice shorter than n currently very conveniently signals EOF. If the slice could be shortened due to the buffer capacity, it would be ambiguous with the EOF case, and require extra logic to detect the EOF or discover the internal buffer capacity, which would make this API much more cumbersome. It could end up being a footgun if authors forget about this edge case, and fail to notice the problem if they only use n < 8K with the default BufReader.

It should not panic.

The buffer capacity is unknown to the caller. A panic would make this API unusable for n > 1 (code needs to be compatible with panic=abort). Also plenty of std APIs that panic ended up getting try_ and _checked non-panicking versions eventually.

It should return explicit information

Unfortunately, the underlying I/O could in theory return any ErrorKind, so it's tricky to return a special kind of io::Error to signal that the capacity is too small. If returning just io::Result<&[u8]> is important for simplicity of this API, maybe the implementation could stretch the definition of ErrorKind::StorageFull, since that one wouldn't usually happen on a read.

I think it should return Option<Result<&[u8]>>. This is an unusual combination, but given how it would be actually used, I think it works quite well:

let slice = bufreader.fill_buf_at_least(n) // peek(n)
    .unwrap_or_else(|| allocate_and_read_into_a_larger_buffer(&mut bufreader, n))?;

This pattern would be very useful in generic contexts where user can supply BufRead with an unknown (and potentially uselessly small) buffer size, while also seamlessly and efficiently handling reading &[u8] via BufRead.

A slice shorter than n currently very conveniently signals EOF.

End of file is typically signaled via a read of length 0, and I don't know why it would need to be any different here.

I think it should return Option<Result<&[u8]>>. This is an unusual combination, but given how it would be actually used, I think it works quite well

I think returning None on what is likely to be programmer error is bad API design.

This pattern would be very useful in generic contexts where user can supply BufRead with an unknown (and potentially uselessly small) buffer size, while also seamlessly and efficiently handling reading &[u8] via BufRead.

peek is currently defined only on BufReader, not on BufRead. If we wanted it to be a trait method, it would require its own trait, as the desired semantics are not possible to achieve using the existing methods of BufRead. The current semantics are not suitable for being a trait (IMO) (not every BufRead has a concept of capacity), so I believe that would require a whole new API design. If you want to design an alternate ACP, by all means do so, but I do not have the time to rework this API from scratch.

It is true that generic code that uses BufReader::peek directly will end up with redundant copies when reading from a bytestring, however, the addition of this method will allow any crate to define a trait that abstracts over BufReader and bytestrings to allow efficient lookahead for both.

Also, you didn't consider one of the options for oversized peeks, which is to automatically expand the capacity of the internal buffer.

Navigation Menu

Repository navigation

Tracking Issue for `bufreader_peek` #128405

Public API

Steps / History

Unresolved Questions

22 remaining items

It should not return a shorter slice.

It should not panic.

It should return explicit information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Tracking Issue for bufreader_peek #128405

Description

Public API

Steps / History

Unresolved Questions

Footnotes

Activity

lolbinarycat commented on Jul 30, 2024

lolbinarycat commented on Aug 28, 2024

22 remaining items

fintelia commented on Mar 31, 2025

lolbinarycat commented on Mar 31, 2025

Kixunil commented on Mar 31, 2025

lolbinarycat commented on Mar 31, 2025

fintelia commented on Mar 31, 2025

Kixunil commented on Mar 31, 2025

lolbinarycat commented on Mar 31, 2025

lolbinarycat commented on Mar 31, 2025

Kixunil commented on Apr 1, 2025

the8472 commented on Apr 29, 2025

lolbinarycat commented on Apr 29, 2025

kornelski commented on May 29, 2025

It should not return a shorter slice.

It should not panic.

It should return explicit information

lolbinarycat commented on May 29, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Tracking Issue for `bufreader_peek` #128405