Skip to content

Conversation

@Berrysoft
Copy link
Member

Closes #590

Users may choose the preferred API set to use - either compio-io or futures-util. All tests and examples are fixed to use compio-io traits. Only the benchmark uses futures-util to align the comparison with quinn.

There might be an argument about removing the previous read and read_exact, because it receives BufMut which allows the users to pass uninitialized buffers. I would like to point out that poll_read_uninit serves this purpose, and users might want to implement hyper::Read or tokio::AsyncRead though it. It's the same as AsyncStream::poll_read_uninit.

@Berrysoft Berrysoft added this to the v0.18 milestone Dec 21, 2025
@Berrysoft Berrysoft self-assigned this Dec 21, 2025
@Berrysoft Berrysoft added refactor Refactoring existing code package: quic Related to compio-quic labels Dec 21, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the QUIC I/O APIs to support both compio-io and futures-util trait implementations, allowing users to choose their preferred API set. The refactoring removes methods that relied on the BufMut trait and introduces new APIs based on uninitialized buffers and owned buffer types.

  • Removed read, read_exact, write, and write_all methods that accepted BufMut arguments
  • Introduced poll_read_uninit for low-level reading into uninitialized buffers
  • Redesigned read_to_end to accept a size_limit parameter instead of a user-provided buffer
  • Updated all tests, examples, and benchmarks to use the new compio-io traits

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
compio-quic/src/recv_stream.rs Replaced poll_read and removed read/read_exact methods; introduced poll_read_uninit; redesigned read_to_end to allocate its own buffer; renamed error BufferTooShort to TooLong
compio-quic/src/send_stream.rs Removed standalone write and write_all methods; inlined write logic into AsyncWrite trait implementation
compio-quic/tests/echo.rs Updated to use new write_all from AsyncWriteExt and new read_to_end signature
compio-quic/tests/basic.rs Updated API calls to use new signatures; demonstrates using both RecvStream::read_to_end and AsyncReadExt::read_to_end for buffer reuse
compio-quic/examples/quic-server.rs Updated imports and read_to_end usage
compio-quic/examples/quic-dispatcher.rs Updated imports and API calls to use new write_all and read_to_end
compio-quic/examples/quic-client.rs Updated imports and read_to_end usage
compio-quic/benches/quic.rs Added futures_util::AsyncWriteExt import for benchmark; updated read_to_end calls
compio-quic/Cargo.toml Added io-compat feature requirement for benchmarks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Berrysoft
Copy link
Member Author

An overhead might be: AsyncReadExt::read_to_end, either the compio-io one or the futures-util one, doesn't optimize for unordered read, and might be slower than RecvStream::read_to_end.

@Berrysoft
Copy link
Member Author

I don't think size_limit is that useful. How about just implementing fn read_to_end<B: IoBufMut>(B) -> BufResult<usize, B>?

@George-Miao George-Miao changed the title refactor(quic): redesign IO APIs refactor(quic)!: redesign IO APIs Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change package: quic Related to compio-quic refactor Refactoring existing code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

compio-quic: IO API redesign?

2 participants