On Thu, Jun 22, 2017 at 08:19:40PM +0200, René Scharfe wrote: > > I'd be OK with keeping it if we could reduce the number of magic > > numbers. E.g,. rather than 32 elsewhere use: > > > > (sizeof(*loose_objects_subdir_bitmap) * CHAR_BIT) > > We have a bitsizeof macro for that. > > > and similarly rather than 8 here use > > > > 256 / sizeof(*loose_objects_subdir_bitmap) / CHAR_BIT > > If we're pretending not to know the number of bits in a byte then we > need to round up, and we have DIV_ROUND_UP for that. :) Thanks, I meant to mention the rounding thing but forgot. I didn't know about either of those macros, though. > There is another example of a bitmap in shallow_c (just search for > "32"). It would benefit from the macros mentioned above. That > might make it easier to switch to the native word size (unsigned int) > instead of using uint32_t everywhere. > > But perhaps this one is actually a candidate for using EWAH, depending > on the number of refs the code is supposed to handle. I thought at first you meant real EWAH bitmaps. But those aren't random access (so you have to read them into an uncompressed bitmap in memory, which is why ewah/bitmap.c exists). But if you just mean reusing those bitmaps, then yeah, possibly. It looks like it's using a commit slab, though. If you know the number of bits you need up front, then the commit slab can do it without any waste. I didn't dig enough to see if that's what it's doing or not. -Peff
