Make zerocopy optional

[mitsuhiko]

Feel free to disregard this. I was wondering if there is a way to make zerocopy optional. It's actually not all that hard, since most of Convert is actually not used. However you end up with a fair bit if not nice looking code which I think is inappropriate compared to what zerocopy achieves.

However that got me thinking that it might be interesting to make zerocopy a CI only thing, or something a user can opt-into. Since all the transmutes are internal, it's shielded from the public API anyways. For as long as the code passes zerocopy checks, the transmutes are fine and zerocopy is not needed for distribution.

Reason for why I want to avoid zerocopy for now: zerocopy is work in progress and I currently end up with more than one zerocopy version in my dependency, in part because ahash has not moved up yet (#246). To avoid some extra churn I was thinking it might be interesting to see if it can be made optional.

Refs #180

[briansmith]

most of Convert is actually not used.

It is likely very few of the convert! macro invocations are necessary. If we were to remove all the unused ones, the decision about what to do would be easier. It is trickier than I expected to find which uses are unused. Still, I submitted PR #254 and PR #255 along these lines.

I also submitted PR #256 to show that some conversions can now be done without transmutes at all.

[briansmith]

@mitsuhiko Any chance you would be interested in submitting a PR or two that removes the unused code you noticed?

[briansmith]

I replaced PR #255 with PR #257. In conjunction with PR #256, this leaves:

convert!([u128; 4], [u8; 64]);
convert!([u128; 2], [u64; 4]);
convert!([u128; 2], [u8; 32]);
convert!(u128, [u64; 2]);
convert_primitive_bytes!(u128, [u8; 16]);
convert!([u64; 2], [u32; 4]);
#[cfg(test)]
convert!([u64; 2], [u8; 16]);
convert_primitive_bytes!(u64, [u8; 8]);
convert_primitive_bytes!(u32, [u8; 4]);
convert_primitive_bytes!(u16, [u8; 2]);
convert!([[u64; 4]; 2], [u8; 64]);

I believe you could manually rewrite the uses of convert! that can't be implemented in terms of convert_primitive_bytes! without using transmutes or other unsafe and (I hope) without performance penalty.

[briansmith]

This still suffers from the fact that the unsafe is hidden in a macro, and also there is no SAFETY comment. I think those were the two problems that the PR that added zerocopy was trying to address.

If it is important to use transmute for performance or other reasons, then it would be better to just manually copy/paste the necessary implementations so that all the unsafe are moved out of the macro.

[mitsuhiko]

Agreed. This would need to be addressed. I just saw that you pushed PRs up that I think in general are a good thing to land if possible. So maybe that should take priority.

I general though I think all of this is stalled for the moment until a maintainer shows up. I'm not sure who has permissions to merge or approve things here.

[tkaitchuck]

If the transmutes can be avoided or tamed with no overhead I am all for that, but I don't want to add a feature flag for it.

[briansmith]

Some PRs that greatly reduce the number of uses of convert! were recently merged. PR #266, PR #270, and PR #271 eliminate some more usages; PR #270 needs performance testing that I won't have time to do. Regardless, all of these little changes make the remaining work much smaller if somebody wants to finish it up along the same lines. I had written many of these little changes in a few hours many weeks ago, but it is too time consuming to rebase and submit them as the weeks go by, so I don't intend to submit more.