TimelyDataflow/abomonation

`impl Abomonation for u/i128`

Closed this issue · 7 comments

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

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?

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.

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

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?

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.

@frankmcsherry I do believe this can be closed now?

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