Skip to content

Add Read::is_append_only method to optimize read_to_string #89711

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

jkugelman
Copy link
Contributor

I've added a new method called is_append_only to the Read trait. It returns true if read_to_end and read_to_string can be trusted not to read or overwrite any existing data in their input buffer.

These readers use it to signal that they have optimized read_to_end methods:

  • [u8] extends the input buffer to the full slice size all at once instead of doubling its size repeatedly until it's big enough.

  • File pre-allocates enough space for the full file size, allowing for fewer read syscalls and reducing buffer reallocations.

These readers take advantage of it:

  • If BufReader wraps a [u8] or File its read_to_string will call their optimized read_to_ends.

Overall, this fixes a slow path where BufReader::read_to_string would be forced to read into a side buffer when passed a non-empty input buffer.

I've added a new method called `is_append_only` to the `Read` trait. It
returns true if `read_to_end` and `read_to_string` can be trusted not to
read or overwrite any existing data in their input buffer.

These readers use it to signal that they have optimized `read_to_end`
methods:

* `[u8]` extends the input buffer to the full slice size all at once
  instead of doubling its size repeatedly until it's big enough.

* `File` pre-allocates enough space for the full file size, allowing for
  fewer `read` syscalls and reducing buffer reallocations.

These readers take advantage of it:

* If `BufReader` wraps a `[u8]` or `File` its `read_to_string` will
  call their optimized `read_to_end`s.

Overall, this fixes a slow path where `BufReader::read_to_string` would
be forced to read into a side buffer when passed a non-empty input
buffer.
@rust-highfive
Copy link
Contributor

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2021
@jkugelman
Copy link
Contributor Author

jkugelman commented Oct 9, 2021

This is what I came up with to fix the slow path identified in #89582 (comment). Introducing a new trait method feels like swatting a fly with a bazooka. I don't have any better ideas, though. (Except, of course, doing nothing.)

cc @a1phyr

///
/// [`read_to_end`]: Read::read_to_end
/// [`read_to_string`]: Read::read_to_string
#[unstable(feature = "append_only", issue = "none")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placeholder for a new feature / tracking issue.

@the8472 the8472 added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Oct 11, 2021
@jkugelman jkugelman marked this pull request as ready for review October 13, 2021 04:08
@jkugelman
Copy link
Contributor Author

I'm going to submit a different idea.

@jkugelman jkugelman closed this Oct 16, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants