Am 18.06.2017 um 15:56 schrieb Jeff King:
On Sun, Jun 18, 2017 at 02:59:04PM +0200, René Scharfe wrote:
@@ -1586,6 +1587,9 @@ extern struct alternate_object_database {
struct strbuf scratch;
size_t base_len;
+ uint32_t loose_objects_subdir_bitmap[8];
Is it worth the complexity of having an actual bitmap and not just an
array of char? I guess it's not _that_ complex to access the bits, but
there are a lot of magic numbers involved.
That would be 224 bytes more per alternate_object_database, and we'd
gain simpler code. Hmm. We could add some bitmap helper macros, but
you're probably right that the first version should use the simplest
form for representing small bitmaps that we currently have.
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. :)
There's also already a bitmap data structure in ewah/bitmap.c. It's a
little bit overkill, though, because it mallocs and will grow the bitmap
as needed.
Yes, I feel that's too big a hammer for this purpose.
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.
+static void read_loose_object_subdir(struct alternate_object_database *alt,
+ int subdir_nr)
I think it's nice to pull this out into a helper function. I do wonder
if it should just be reusing for_each_loose_file_in_objdir(). You'd just
need to expose the in_obj_subdir() variant publicly.
It does do slightly more than we need (for the callbacks it actually
builds the filename), but I doubt memcpy()ing a few bytes would be
measurable.
Good point. The function also copies the common first two hex digits
for each entry. But all that extra work is certainly dwarfed by the
readdir calls.
Yes. You're welcome to micro-optimize that implementation if you want. ;)
My knee-jerk reaction was that this would lead to ugliness, but on
second look it might actually be doable. Will check, but I don't
expect much of a speedup.
René
