search

TimelyDataflow / abomonation

A mortifying serialization library for Rust
MIT License
317 stars 30 forks source link

`impl Abomonation for u/i128` #11

Closed ryzhyk closed 5 years ago

ryzhyk commented 5 years ago

128-bit integers are not abomonable at the moment. Is there a reason not to support this?

frankmcsherry commented 5 years ago

No reason, except they stabilized relatively recently (and I've never used one). We should be able to just add it to the list of implemented things. Any others come to mind?

frankmcsherry commented 5 years ago

Actually, there could be an issue if either of these tickle alignment issues. They possibly end up in SIMD registers, and misaligned loads/stores could be actual panics there rather than just perf overhead like it is for other types. I'll implement them and study, and PR if it seems they work.

frankmcsherry commented 5 years ago

Well, I just tried and .. nothing crashed. The test that serializes, but with a one character string serialized before the u128, ran and confirmed that the decoded result == the original. Um. Any thoughts on whether that is sufficient? :D

ryzhyk commented 5 years ago

No idea, tbh. I know that 128-bit integers can be implemented in different ways on different platforms, so I'm not sure what a complete test matrix should look like. But shouldn't it be the compiler's job to make sure that data is properly aligned (perhaps by making an extra copy) before using it in SIMD instructions?

frankmcsherry commented 5 years ago

Apropos abomonation support, timely just landed a bincode feature flag that should allow you to use any types that implement Serialize+for<'a>Deserialize<'a>, rather than requiring Abomonation. At the moment this includes e.g. HashMap, probably BTree, maybe some other things. Unfortunately, you still cannot use f64 types in differential, because they do not implement Ord.

benesch commented 5 years ago

@frankmcsherry I do believe this can be closed now?

frankmcsherry commented 5 years ago

Yup, it should be good to go (tests recently added, but no surprises there).