recv multi with panic #253
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds new types: bufring::{BufRing, Builder}, bufgroup:{BufX, Bgid, Bid}
(BufX was chosen as the name of the buffer type of this pool
simply to make it short and easy to grep for.)
bufgroup may be better named provbuf, or provided_buffers, down the
road, especially if we intent to support the other, older, much less
efficient provided buffer mechanism. But the liburing team
specifically recommends the buf_ring implementation for ease of use
and performance reasons so we may not want to incur the overhead of
supporting both forms.
bufring could become a trait down the road, where different BufRing
implementations can be provided. But to avoid the extra decision
making that comes with designing traits, and to keep the code
relatively easy to follow, these are all concrate types.
Adds register and unregister functionality to BufRing and to the
tokio_uring driver.
Adds experimental recv, recv_provbuf methods to TcpStream.
recv_provbuf is a purposefully ugly name. It will be replaced by
something ... maybe a recv builder sometimme soon.
Whatever is chosen would then be copied for Udp, Unix and for all
the other operations that optionally take a provided buffer pool id.
Adds 'net' unit tests. All are admittedly simple ping/pong tests where
only the clients' received lengths are checked, not the actual data.
Adds a tests/common area.
Adds a test case that uses two std::threads, where each thread runs its
own tokio_uring runtime and its own buf_ring provided buffer pool.
The two-thread case is made long, with many clients, sending many
large messages to the server and getting them back, in order to see
gross performance impacts when changing things. It takes 3s on my
machine. Before going into mainline, the numbers would be changed so
it took no more than the other unit tests, so about 10ms.
Many TODOs left to cleanup. Primarily Safety rationalizations.
The buffer allocation is made as a single allocation, along with the
ring.
The buffer group id, bgid, also sometimes called the provided buffer
group id, is manually selected through the builder api. There is no
mechanism to pick one automatically. That could be added later but is
not really necessary for this feature to be useful.
This first implementation is without traits and without public
interfaces that would let a user create a different kind of buf_ring or
a different kind of `provided buffers` pool.
There's a question to the liburing team outstanding about how to
interpret an unexpected cqe result of res=0 and flags=4.
The first operation that supports streaming CQE results.
Adds a Streamable trait, along the lines of the Completable trait.
Comes with stream_next and stream_complete methods.
Adds MultiCQEStream struct, along the lines of the MultiCQEFuture
struct.
Adds a submit_op_stream along the lines of submit_op.
Adds poll_next_op along the lines of poll_op.
The Lifecycle gets two additional methods: take_index, and data.
take_index: returns the Lifecycle index and replaces it with
usize::MAX to show the Lifecycle is no longer represented in the
slab. This feature only used by the new poll_next_op.
data: returns a reference to the Lifecycle's data, to be able to use
it for the Streamable's stream_next calls which only require a
reference. Ownership is still transferred in stream_complete.
Adds the io/recv_multi operation.
The tcp stream recv_multi is a function that
bridges the private types with a public function.
There is no cancel yet for the multishot command but the code can be
written to break out of the loop. Also, when the connection is closed,
the command should fail. It's not tested, but unregistering the buf_ring
might cancel the command too - but maybe not.
Unit tests:
net unit tests and helper functions return io::Return
add recv_multi cases
add BufRingProps to net unit tests - to quiet empty buffer warnings
which are intentional in some tests
|
At the top of this page, is the unfortunate wording This is a draft, there is no merge wanted. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a draft, to show the call to
self.inner.borrow_mut().resubmit_op_streamin runtime/driver/handle.rs and why it seems desirable to be able to push to the io_uring squeue from within the context of a Future or Stream being polled. It's all synchronous, but because the driver does not support mutable access to the io-uring's squeue without having a mutable reference, and the mutable reference appears to be held at a higher level while task polls are going on, this is a problem.There are a lot of commits, but just the latest one is made over the recv_multi PR.