-
Notifications
You must be signed in to change notification settings - Fork 79
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
Binary serialization over pipes #741
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #741 +/- ##
=======================================
Coverage ? 55.28%
=======================================
Files ? 72
Lines ? 9835
Branches ? 0
=======================================
Hits ? 5437
Misses ? 4398
Partials ? 0
☔ View full report in Codecov by Sentry. |
61f709d
to
a62a216
Compare
Thank you for your review ! I ran into an issue I feel like is blocking, which I couldn't work around (see the review above). If there's no good work around I'm okay closing this PR, but let me know. |
@xy2i oh silly me... Yeah I forgot that const generics is very minimal right now. But this is no reason to not merge your PR. There might be an alternative but I'm not sure how well it works: mod sealed {
pub trait DeSerializeBytes {}
impl<const N: usize> DeSerializeBytes for [u8; N] {}
}
pub trait DeSerialize {
type Bytes: sealed::DeSerializeBytes;
fn serialize(&self) -> Self::Bytes;
fn deserialize(bytes: Self::Bytes) -> Self;
}
impl DeSerialize for i32 {
type Bytes = [u8; std::mem::size_of::<Self>()];
fn serialize(&self) -> Self::Bytes {
self.to_ne_bytes()
}
fn deserialize(bytes: Self::Bytes) -> Self {
Self::from_ne_bytes(bytes)
}
} Let me know if you think that's good enough. Otherwise, we can merge this as it is I think :) Thanks! |
That works, thanks ! |
We use `UnixStream::pair()` a few times and reimplement binary ser/de each time. Add an abstraction that does the serde steps (`DeSerialize`) and buffer ceremony (`BinPipe`) for us. Fixes trifectatechfoundation#471.
Thanks for this! |
5165038
We use
UnixStream::pair()
a few times and reimplement binary ser/de each time. Add an abstraction that does the serde (BinSerDe
) and buffer ceremony (BinPipe
) for us.Fixes #471.