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

Introduce SharedBuf trait for Bytes VTable #596

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

rrichardson
Copy link
Contributor

The goal of this PR is to continue the work of PR #567. This updates the code to be more conformant to the idioms of Rust and the Tokio project.

The work in #567 introduces a new trait which is used as a contract for internal and external structs to make themselves usable as the internal buffer for Bytes.

This should solve the current issues that request a mechanism for allowing a Bytes struct to own arbitrary 3rd party buffers. see: #437 #526 #571

This would allow an arbitrary buffer to be converted into a Bytes buffer (using the new Bytes::from_shared_buf method) and later be converted back to the original type, consuming the buffer (using Bytes::into_shared_buf method)
An example of this can be found here

Specific changes from #567:

  • Renames the trait BytesImpl into SharedBuf.
  • Modularizes the implementations of SharedBuf and moves them into their own module.
  • Changes some types to ensure that theSharedBuf trait can be implemented by 3rd party devs.
  • Adds an example "3rd party" SharedBuf implementation with tests.
  • Adds a bunch of documentation.

Due to re-organizing the SharedBuf implementations, this makes things a bit harder to diff, but the functionality of Bytes itself is almost completely unchanged. All tests pass, and the benchmarks are equivalent within a reasonable deviation (using cargo benchcmp)

For more precise deltas between the code, you can look at the commit progression in #567

  1. change to_vec to into_vec and takes &mut AtomicPtr<()> instead of &At…
  2. add will_truncate to Bytes vtable to prevent special casing promotabl…
  3. add Bytes::with_impl and related trait
  4. migrate existing vtable code to BytesImpl trait

@Darksonn Darksonn requested a review from seanmonstar February 7, 2023 10:35
@rrichardson
Copy link
Contributor Author

I've addressed the formatting, and will address the loom issue. The miri issue is an interesting one, I'm having trouble divining the origin of that one.

@carllerche
Copy link
Member

Thanks for the PR. We will be reviewing it soon. Given the size, it will take a moment to get through it.

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.

3 participants