refactor(s2n-quic-dc): shared caches improvements #2883
Conversation
dc/s2n-quic-dc/src/uds/sender.rs
Outdated
| let this = self.get_mut(); | ||
|
|
||
| loop { | ||
| let mut guard = ready!(this.sender.socket_fd.poll_write_ready(cx))?; |
There was a problem hiding this comment.
From tokio docs about AsyncFd::poll_write_ready,
"Because these functions don’t create a future to hold their state, they have the limitation that only one task can wait on each direction (read or write) at a time."
Would this become an issue if we have multiple acceptor tasks sending to the same unix domain socket?
There was a problem hiding this comment.
Yes. We'll need to see if we can use writable and perhaps have a fixed size allocation for the returned future... I'm not seeing a non-poll API with a named future return type in the tokio API.
dc/s2n-quic-dc/src/uds/sender.rs
Outdated
| fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> { | ||
| let this = self.get_mut(); | ||
|
|
||
| loop { |
There was a problem hiding this comment.
From the docs, "On some platforms, the readiness detecting mechanism relies on edge-triggered notifications. This means that the OS will only notify Tokio when the file descriptor transitions from not-ready to ready. For this to work you should first try to read or write and only poll for readiness if that fails with an error of std::io::ErrorKind::WouldBlock."
Is this something we care about? I think this would mean we should try to send on the socket first.
mehnazyunus
force-pushed
the
improvements
branch
from
c1caaba to
06ea32c
Compare
| .unwrap(); | ||
| info!("Handshake completed"); | ||
|
|
||
| let app_server = create_application_server(&unix_socket_path, test_event_subscriber.clone()); |
There was a problem hiding this comment.
uds sender is now created on manager build (instead of on accepting a stream from the client), so we need the application receive socket to be created first.
dc/s2n-quic-dc/src/uds/sender.rs
Outdated
| // Allocate a future on the heap only if initial send returns WouldBlock | ||
| let sender = this.sender.clone(); | ||
| let packet = this.packet.clone(); | ||
| let fd = this.fd_to_send.try_clone()?; |
There was a problem hiding this comment.
I think rather than cloning these, we'd probably want to make SendMsg an enum and switch it to a Future state, i.e., it would move out of these fields?
dc/s2n-quic-dc/src/uds/sender.rs
Outdated
| pub fn new(sender: Sender, packet: &[u8], fd_to_send: OwnedFd) -> SendMsg { | ||
| SendMsg { | ||
| sender, | ||
| packet: packet.to_vec(), |
There was a problem hiding this comment.
I think the caller already has a Vec, can they just provide that to us?
3 participants
Release Summary:
Resolved issues:
Description of changes:
Call-outs:
Added in comments.
Testing:
Checked that existing tests pass and ensured that CPU utilization is 0 when adding a sleep after sending streams in
test_kernel_queue_full.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.