Tracking Issue for std::io::read_to_string
#80218
Comments
camelid
changed the title
io::read_to_stringstd::io::read_to_string
camelid
added
C-tracking-issue
m-ou-se
added
the
Libs-Tracked
|
What is the status on stabilizing this addition to the library? |
|
@m-ou-se / @rust-lang/libs Can we start an FCP? |
|
Thinking a bit about the signature: Considering |
I modeled it after |
|
I'm working on a related PR #89582 to have If it lands do you think there's anything in this comment that should be updated?
|
That's great; thanks for working on this!
Perhaps we could add another paragraph at the end like this:
|
|
Done. |
|
It seems like Would it be |
Reading a file into an empty vector or string buffer can incur unnecessary `read` syscalls and memory re-allocations as the buffer "warms up" and grows to its final size. This is perhaps a necessary evil with generic readers, but files can be read in smarter by checking the file size and reserving that much capacity. `std::fs::read` and `read_to_string` already perform this optimization: they open the file, reads its metadata, and call `with_capacity` with the file size. This ensures that the buffer does not need to be resized and an initial string of small `read` syscalls. However, if a user opens the `File` themselves and calls `file.read_to_end` or `file.read_to_string` they do not get this optimization. ```rust let mut buf = Vec::new(); file.read_to_end(&mut buf)?; ``` I searched through this project's codebase and even here are a *lot* of examples of this. They're found all over in unit tests, which isn't a big deal, but there are also several real instances in the compiler and in Cargo. I've documented the ones I found in a comment here: rust-lang#89516 (comment) Most telling, the `Read` trait and the `read_to_end` method both show this exact pattern as examples of how to use readers. What this says to me is that this shouldn't be solved by simply fixing the instances of it in this codebase. If it's here it's certain to be prevalent in the wider Rust ecosystem. To that end, this commit adds specializations of `read_to_end` and `read_to_string` directly on `File`. This way it's no longer a minor footgun to start with an empty buffer when reading a file in. A nice side effect of this change is that code that accesses a `File` as a bare `Read` constraint or via a `dyn Read` trait object will benefit. For example, this code from `compiler/rustc_serialize/src/json.rs`: ```rust pub fn from_reader(rdr: &mut dyn Read) -> Result<Json, BuilderError> { let mut contents = Vec::new(); match rdr.read_to_end(&mut contents) { ``` Related changes: - I also added specializations to `BufReader` to delegate to `self.inner`'s methods. That way it can call `File`'s optimized implementations if the inner reader is a file. - The private `std::io::append_to_string` function is now marked `unsafe`. - `File::read_to_string` being more efficient means that the performance note for `io::read_to_string` can be softened. I've added @camelid's suggested wording from: rust-lang#80218 (comment)
…r=joshtriplett Optimize File::read_to_end and read_to_string Reading a file into an empty vector or string buffer can incur unnecessary `read` syscalls and memory re-allocations as the buffer "warms up" and grows to its final size. This is perhaps a necessary evil with generic readers, but files can be read in smarter by checking the file size and reserving that much capacity. `std::fs::read` and `std::fs::read_to_string` already perform this optimization: they open the file, reads its metadata, and call `with_capacity` with the file size. This ensures that the buffer does not need to be resized and an initial string of small `read` syscalls. However, if a user opens the `File` themselves and calls `file.read_to_end` or `file.read_to_string` they do not get this optimization. ```rust let mut buf = Vec::new(); file.read_to_end(&mut buf)?; ``` I searched through this project's codebase and even here are a *lot* of examples of this. They're found all over in unit tests, which isn't a big deal, but there are also several real instances in the compiler and in Cargo. I've documented the ones I found in a comment here: rust-lang#89516 (comment) Most telling, the documentation for both the `Read` trait and the `Read::read_to_end` method both show this exact pattern as examples of how to use readers. What this says to me is that this shouldn't be solved by simply fixing the instances of it in this codebase. If it's here it's certain to be prevalent in the wider Rust ecosystem. To that end, this commit adds specializations of `read_to_end` and `read_to_string` directly on `File`. This way it's no longer a minor footgun to start with an empty buffer when reading a file in. A nice side effect of this change is that code that accesses a `File` as `impl Read` or `dyn Read` will benefit. For example, this code from `compiler/rustc_serialize/src/json.rs`: ```rust pub fn from_reader(rdr: &mut dyn Read) -> Result<Json, BuilderError> { let mut contents = Vec::new(); match rdr.read_to_end(&mut contents) { ``` Related changes: - I also added specializations to `BufReader` to delegate to `self.inner`'s methods. That way it can call `File`'s optimized implementations if the inner reader is a file. - The private `std::io::append_to_string` function is now marked `unsafe`. - `File::read_to_string` being more efficient means that the performance note for `io::read_to_string` can be softened. I've added `@camelid's` suggested wording from rust-lang#80218 (comment). r? `@joshtriplett`
`@m-ou-se` [realized][1] that because `Read` is implemented for `&mut impl Read`, there's no need to take `&mut` in `io::read_to_string`. Removing the `&mut` from the signature allows users to remove the `&mut` from their calls (and thus pass an owned reader) if they don't use the reader later. [1]: rust-lang#80218 (comment)
|
Now that the PR to remove the |
…ou-se Remove `&mut` from `io::read_to_string` signature ``@m-ou-se`` [realized][1] that because `Read` is implemented for `&mut impl Read`, there's no need to take `&mut` in `io::read_to_string`. Removing the `&mut` from the signature allows users to remove the `&mut` from their calls (and thus pass an owned reader) if they don't use the reader later. r? `@m-ou-se` [1]: rust-lang#80218 (comment)
|
I will open a stabilization PR next week or so. |
|
Actually, I think a libs team FCP needs to be completed first. @m-ou-se could you kick that off? (Or do I need to do something before that?) |
|
@rfcbot merge |
|
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
rfcbot
added
the
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
rfcbot
removed
the
proposed-final-comment-period
rfcbot
added
finished-final-comment-period
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
…g, r=JohnTitor Stabilize `std::io::read_to_string` Closes rust-lang#80218. 🎉 **Blocked on FCP finishing**
…g, r=JohnTitor Stabilize `std::io::read_to_string` Closes rust-lang#80218. 🎉 This PR stabilizes the `std::io::read_to_string` function, with the following public API: ```rust pub fn read_to_string<R: Read>(reader: R) -> Result<String>; ``` It's analogous to `std::fs::read_to_string` for files, but it works on anything that implements `io::Read`, including `io::stdin()`. See the tracking issue (rust-lang#80218) or documentation for details.
…g, r=JohnTitor Stabilize `std::io::read_to_string` Closes rust-lang#80218. 🎉 This PR stabilizes the `std::io::read_to_string` function, with the following public API: ```rust pub fn read_to_string<R: Read>(reader: R) -> Result<String>; ``` It's analogous to `std::fs::read_to_string` for files, but it works on anything that implements `io::Read`, including `io::stdin()`. See the tracking issue (rust-lang#80218) or documentation for details.
Feature gate:
#![feature(io_read_to_string)]This is a tracking issue for the
std::io::read_to_stringfunction.The equivalent of
std::fs::read_to_string, but generalized to allReadimpls.As the documentation on
std::io::read_to_stringsays, the advantage ofthis function is that it means you don't have to create a variable first
and it provides more type safety since you can only get the buffer out
if there were no errors. If you use
Read::read_to_string, you have toremember to check whether the read succeeded because otherwise your
buffer will be empty.
It's friendlier to newcomers and better in most cases to use an explicit
return value instead of an out parameter.
Public API
Steps / History
std::io::read_to_stringfunction #80217std::io::read_to_string#100337)Unresolved Questions
&mutfromio::read_to_stringsignature #92863)The text was updated successfully, but these errors were encountered: