-
Notifications
You must be signed in to change notification settings - Fork 82
refactor(quic)!: redesign IO APIs #593
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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, andwrite_allmethods that acceptedBufMutarguments - Introduced
poll_read_uninitfor low-level reading into uninitialized buffers - Redesigned
read_to_endto accept asize_limitparameter instead of a user-provided buffer - Updated all tests, examples, and benchmarks to use the new
compio-iotraits
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.
|
An overhead might be: |
|
I don't think |
Closes #590
Users may choose the preferred API set to use - either
compio-ioorfutures-util. All tests and examples are fixed to usecompio-iotraits. Only the benchmark usesfutures-utilto align the comparison withquinn.There might be an argument about removing the previous
readandread_exact, because it receivesBufMutwhich allows the users to pass uninitialized buffers. I would like to point out thatpoll_read_uninitserves this purpose, and users might want to implementhyper::Readortokio::AsyncReadthough it. It's the same asAsyncStream::poll_read_uninit.