On Mon, Jul 31, 2017 at 4:43 PM, Shawn Pearce <spea...@spearce.org> wrote: > On Mon, Jul 31, 2017 at 12:42 PM, Junio C Hamano <gits...@pobox.com> wrote: >> >> As a block cannot be longer than 16MB, allocating uint32 to a >> restart offset may be a bit overkill. I do not know if it is worth >> attempting to pack 1/3 more restart entries in a block by using >> uint24, though. > > I don't think its worth the annoyance in code that has to deal with > this field. An example 87k ref repository with a 4k block size has ~9 > restarts per block and ~19 bytes of padding. Using uint24 saves us 9 > bytes, yet the average ref here costs 27 bytes. We aren't likely to > fill the block with another ref, or another restart point.
I thought about this more. We can fit an additional ref per block in some cases. It saves ~20 KiB for one of my example repositories if restart_offset is a uint24. Given that readers have to deal with these being unaligned loads anyway, its not significantly harder to work with uint24 vs. uint32. So I've changed the definition to be uint24.
