icu4x icon indicating copy to clipboard operation
icu4x copied to clipboard

Allow ULEs to be bit-packed rather than byte-packed

Open sffc opened this issue 3 years ago • 6 comments

Currently, all ULE values are aligned to a byte boundary. We should consider making them aligned to a bit boundary instead.

This would solve two problems:

  1. Inefficient storage: for example, Script requires 10 bits, which means it needs to use a 16-bit unit, wasting 37% of space. (#1404)
  2. Inability for the upcoming #[derive(ULE)] to efficiently pack a struct containing many small fields. (#1079)

There are crates like bitpacking which may provide some prior art. However, I believe that our bitpacking can be less sophisticated, since ZeroVec already returns stack copies of elements in the array.

This may be suited to be a new type, like ZeroVecPacked, rather than re-writing the current class, which is optimized for its current byte-aligned use case.

CC @Manishearth

sffc avatar Jan 09 '22 06:01 sffc

I'm fine with a new type, strongly against making this change to ZeroVec itself since it would trade off performance for size in a much stronger way. We could even make this work in ZeroMap by introducing a Packed wrapper type (i.e. ZeroMap<Packed<Script>, str>)

To me, the primary value proposition of ZeroVec is zero-copy deserialization, efficient encoding is nice to have but if we're trading off performance for it I'd prefer not to.

It's going to be tricky to implement; I suspect what we'll need to do is:

  • add a PACKED_BITS const to ULE (we already plan to add BITS)
  • the ULE trait invariants also say that the first PACKED_BITS bits of Self are the important ones, if there are any leftover they must be zero.
  • ZeroVecPacked has a slightly different internal implementation that does not explicitly store ULEs and does more on-the-fly transmutes

Manishearth avatar Jan 09 '22 07:01 Manishearth

To me, the primary value proposition of ZeroVec is zero-copy deserialization, efficient encoding is nice to have but if we're trading off performance for it I'd prefer not to.

I agree with this.

iainireland avatar Jan 10 '22 17:01 iainireland

As part of the work on this issue, the performance impact of bitpacking should be measured, and weighed against the resulting impact on data size.

sffc avatar Jan 10 '22 19:01 sffc

As a step in this direction, we could add a byte-sized type that is valid for only certain bits, which could be composed into a ULE type, such that the only manual impl required for bit-packing is a (safe) AsULE impl. I think the safe transmute people are working on something like this.

sffc avatar Mar 14 '24 13:03 sffc