Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Replace internal buffer in decoder with BufRead #231

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kixunil
Copy link

@Kixunil Kixunil commented Mar 31, 2023

The DecoderReader used an internal buffer for reading which came with a number of disadvantages, such as:

  • Needless copying from in-memory readers (slices)
  • Double-buffering already-buffered readers
  • The bytes are lost when dropping or calling into_inner
  • BUF_SIZE is not configurable
  • std::io::BufReader has access to some unstable optimizations which this crate cannot use
  • Reinvents the wheel; there already is std::io::BufReader

This change removes it and requires BufRead instead. Decoding is implemented on top of fill_buf for larger chunks with fallback to small, stack-allocated buffer for tiny chunks that may appear at boundaries.

To resolve borrowing problems decode_to_buf had to be removed which also enabled decoding bytes directly into internal buffer when the buffer to be filled is small rather than into a temporary buffer which is then copied.

This improves performance of reading by around 7-22% on my machine.

Closes #230

The `DecoderReader` used an internal buffer for reading which came with
a number of disadvantages, such as:

* Needless copying from in-memory readers (slices)
* Double-buffering already-buffered readers
* The bytes are lost when dropping or calling `into_inner`
* `BUF_SIZE` is not configurable
* `std::io::BufReader` has access to some unstable optimizations which
  this crate cannot use
* Reinvents the wheel; there already is `std::io::BufReader`

This change removes it and requires `BufRead` instead. Decoding is
implemented on top of `fill_buf` for larger chunks with fallback to
small, stack-allocated buffer for tiny chunks that may appear at
boundaries.

To resolve borrowing problems `decode_to_buf` had to be removed which
also enabled decoding bytes directly into internal buffer when the
buffer to be filled is small rather than into a temporary buffer which
is then copied.

This improves performance of reading by around 7-22% on my machine.
@marshallpierce
Copy link
Owner

Thanks for the contribution. It will probably take me some time to review it, but I'll get to it when I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DecoderReader probably should accept BufRead instead of Read
2 participants