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

Buffer pool tracking issue #365

Closed
Berrysoft opened this issue Dec 5, 2024 · 10 comments
Closed

Buffer pool tracking issue #365

Berrysoft opened this issue Dec 5, 2024 · 10 comments
Assignees
Labels
help wanted Extra attention is needed package: driver Related to compio-driver package: io Related to compio-io package: runtime Related to compio-runtime
Milestone

Comments

@Berrysoft
Copy link
Member

Berrysoft commented Dec 5, 2024

Prior Art

In Progress

Any other advice or design?

@Berrysoft Berrysoft added the package: io Related to compio-io label Dec 5, 2024
@Berrysoft Berrysoft added this to the v0.14 milestone Dec 5, 2024
@Berrysoft
Copy link
Member Author

API of tokio-uring: https://docs.rs/tokio-uring/latest/tokio_uring/net/struct.TcpStream.html#method.read_fixed

The user get the buffer from the registry directly and pass the ownership to the read method.

@George-Miao George-Miao changed the title buffer pool traits design Buffer pool traits design Dec 5, 2024
@George-Miao George-Miao added the help wanted Extra attention is needed label Dec 5, 2024
@Berrysoft
Copy link
Member Author

Also note that there's write_fixed in tokio-uring

@Berrysoft
Copy link
Member Author

Note about io-uring internals:

Buffer ring is a newer feature. It lets the kernel to choose an available buffer, which means the users cannot choose the buffer themselves, and should return the buffer after consuming it.

@Berrysoft
Copy link
Member Author

The returned buffer might be better if it's owned:

pub trait AsyncReadManaged {
    type BufferPool;
    type Buffer: Deref<Target = [u8]>;

    async fn read_managed(
        &mut self,
        buffer_pool: &Self::BufferPool,
        len: usize,
    ) -> IoResult<Self::Buffer>;
}

The inner buffer pool might be Rc.

@George-Miao
Copy link
Member

@Berrysoft Can you provide some examples? I'm not entirely sure how this is supposed to be used.

@Berrysoft
Copy link
Member Author

The usage is like this:

let pool = BufferPool::new(/* count: */16, /* buffer len: */1024).unwrap();
let mut stream = TcpStream::connect(addr).await.unwrap();
// Use len = 0 to make the driver select one.
let buffer = stream.read_managed(&pool, 0).await.unwrap();
// Use buffer, and drop it to return it to the pool.

@George-Miao
Copy link
Member

@Berrysoft So every IO depends on some specific type of BufferPool, is that right? If users want abstraction, they need to do it themselves.

@Berrysoft
Copy link
Member Author

I think so. The buffer count & buffer len of the buffer pool is controled by the user, and they can even create many pools if they want. compio-runtime could provide a specific pool type, and TcpStream will only impl AsyncReadManaged based on compio_runtime::BufferPool.

@George-Miao
Copy link
Member

George-Miao commented Dec 19, 2024

What about this:

trait BufferPool {
    type Buffer;
}

trait AsyncReadManaged {
    type Pool: BufferPool + ?Sized;
    
    async fn read_managed(
        &mut self,
        buffer_pool: &Self::Pool,
        len: usize,
    ) -> IoResult<<Self::Pool as BufferPool>::Buffer>;
}

It's still flexibile (i.e., no other constraint on buffer pool, hell it can even be !Sized)

@George-Miao George-Miao changed the title Buffer pool traits design Buffer pool tracking issue Dec 19, 2024
@George-Miao George-Miao added package: driver Related to compio-driver package: runtime Related to compio-runtime labels Dec 19, 2024
@Berrysoft
Copy link
Member Author

What about this:

I have thought about this design, but didn't choose it because, there might be a case that the implementer of AsyncReadManaged wants to use a different Buffer type other than the one returned by BufferPool. If type Buffer is an assiciated type of trait BufferPool, the implementer need to author a new pool wrapper type to achieve this goal, which is more complex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed package: driver Related to compio-driver package: io Related to compio-io package: runtime Related to compio-runtime
Projects
None yet
Development

No branches or pull requests

2 participants