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

make available the IoBuf impl for bytes::Bytes #43

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

problame
Copy link
Collaborator

@problame problame commented Feb 7, 2024

@problame problame requested a review from koivunej February 7, 2024 12:33
@problame problame changed the title add some convenience for working with Slice<T> add some convenience for working with Slice<T> Feb 7, 2024
@problame problame removed the request for review from koivunej February 7, 2024 13:06
@problame problame marked this pull request as draft February 7, 2024 13:06
@problame problame changed the title add some convenience for working with Slice<T> impl IoBuf for arrays Feb 7, 2024
@problame problame changed the title impl IoBuf for arrays make available more IoBuf impls Feb 7, 2024
@problame problame force-pushed the problame/iobuf-and-slice-conveniences branch from 05523ae to dbf93e6 Compare February 7, 2024 14:50
Co-authored-by: Joonas Koivunen <[email protected]>
@problame problame changed the title make available more IoBuf impls make available the IoBuf impl for bytes::Bytes Feb 7, 2024
@problame problame marked this pull request as ready for review February 8, 2024 09:35
Copy link
Member

@koivunej koivunej left a comment

Choose a reason for hiding this comment

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

Ok it was already implemented, this is just about exposing it.

@problame problame merged commit 27e56fd into main Feb 8, 2024
3 checks passed
@problame problame deleted the problame/iobuf-and-slice-conveniences branch February 8, 2024 10:51
problame added a commit to neondatabase/neon that referenced this pull request Feb 12, 2024
This PR refactors the `blob_io` code away from using slices towards
taking owned buffers and return them after use.
Using owned buffers will eventually allow us to use io_uring for writes.

part of #6663

Depends on neondatabase/tokio-epoll-uring#43

The high level scheme is as follows:
- call writing functions with the `BoundedBuf`
- return the underlying `BoundedBuf::Buf` for potential reuse in the
caller

NB: Invoking `BoundedBuf::slice(..)` will return a slice that _includes
the uninitialized portion of `BoundedBuf`_.
I.e., the portion between `bytes_init()` and `bytes_total()`.
It's a safe API that actually permits access to uninitialized memory.
Not great.

Another wrinkle is that it panics if the range has length 0.

However, I don't want to switch away from the `BoundedBuf` API, since
it's what tokio-uring uses.
We can always weed this out later by replacing `BoundedBuf` with our own
type.
Created an issue so we don't forget:
neondatabase/tokio-epoll-uring#46
jcsp pushed a commit to neondatabase/neon that referenced this pull request Feb 12, 2024
This PR refactors the `blob_io` code away from using slices towards
taking owned buffers and return them after use.
Using owned buffers will eventually allow us to use io_uring for writes.

part of #6663

Depends on neondatabase/tokio-epoll-uring#43

The high level scheme is as follows:
- call writing functions with the `BoundedBuf`
- return the underlying `BoundedBuf::Buf` for potential reuse in the
caller

NB: Invoking `BoundedBuf::slice(..)` will return a slice that _includes
the uninitialized portion of `BoundedBuf`_.
I.e., the portion between `bytes_init()` and `bytes_total()`.
It's a safe API that actually permits access to uninitialized memory.
Not great.

Another wrinkle is that it panics if the range has length 0.

However, I don't want to switch away from the `BoundedBuf` API, since
it's what tokio-uring uses.
We can always weed this out later by replacing `BoundedBuf` with our own
type.
Created an issue so we don't forget:
neondatabase/tokio-epoll-uring#46
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.

2 participants