feat(s2n-quic-core): add buffer::Deque implemention #2207
Conversation
camshaft
force-pushed
the
camshaft/buffer-deque
branch
from
b01d421 to
6eb9040
Compare
| impl Deque { | ||
| #[inline] | ||
| pub fn new(mut cap: usize) -> Self { | ||
| if !cap.is_power_of_two() { |
There was a problem hiding this comment.
No real need for a conditional -- "Returns the smallest power of two greater than or equal to self." is the documentation for next_power_of_two, so it's a no-op if you're already a power of two.
| send: Data::new(usize::MAX as _), | ||
| recv: Data::new(usize::MAX as _), |
There was a problem hiding this comment.
Why the customization vs. Data::default()?
There was a problem hiding this comment.
Default should work, yeah
There was a problem hiding this comment.
Oh right. The Default implementation for Data is set to 1024 bytes and I wanted an endless stream in this case.
| impl Default for Model { | ||
| fn default() -> Self { | ||
| Self { | ||
| buffer: Deque::new(u16::MAX as _), |
There was a problem hiding this comment.
Should we be testing a smaller Deque? That way we're more likely to notice problems around the edges, right? (And tests should be faster).
Not sure if we're running this through kani though, in which case maybe it matters less. Ideally there we'd do a kani::any() or something here?
There was a problem hiding this comment.
Yeah that's probably a good call. I guess the size doesn't matter too much as long as we're exercising the corner cases. Let me try and get the Kani harness working as well.
| let oracle = { | ||
| let (head, tail) = self.oracle.as_slices(); | ||
| head.iter().chain(tail) | ||
| }; |
There was a problem hiding this comment.
Should we check that we also match the head/tail lengths? Or do we expect our impl to differ there?
There was a problem hiding this comment.
I didn't want to over index on the impls strictly matching so I decided against that.
| std::io::IoSlice::new(&[]), | ||
| split_at, | ||
| |chunk: &[u8]| unsafe { | ||
| // SAFETY: we're using transmute to extend the lifetime of the chunk to `self` |
There was a problem hiding this comment.
| // SAFETY: we're using transmute to extend the lifetime of the chunk to `self` | |
| // SAFETY: we're using transmute to extend the lifetime of the chunk to `self` | |
| // Upstream tracking: https://github.com/rust-lang/rust/issues/124659 |
Description of changes:
This adds a byte ring buffer for efficient vectored reads/writes between socket syscalls.
Testing:
I've added several bolero harnesses for checking the correctness of the implementation.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.