Re: [PATCH 3/5] make object_directory.loose_objects_subdir_seen a bitmap
From: Eric Wong <hidden>
Date: 2021-06-28 23:09:56
Subsystem:
the rest · Maintainer:
Linus Torvalds
René Scharfe [off-list ref] wrote:
Am 27.06.21 um 04:47 schrieb Eric Wong: Anyway, it would look something like this: size_t word_bits = bitsizeof(odb->loose_objects_subdir_seen[0]); size_t word_index = subdir_nr / word_bits; size_t mask = 1 << (subdir_nr % word_bits);
<snip> yeah, I missed bitsizeof :x
quoted
--- a/object-store.h +++ b/object-store.h@@ -22,7 +22,7 @@ struct object_directory { * * Be sure to call odb_load_loose_cache() before using. */ - char loose_objects_subdir_seen[256]; + uint32_t loose_objects_subdir_seen[8]; /* 256 bits */Perhaps DIV_ROUND_UP(256, bitsizeof(uint32_t))? The comment explains it nicely already, though.
I think I'll keep my original there... IMHO the macros obfuscate the meaning for those less familiar with our codebase (myself included :x)
quoted
struct oid_array loose_objects_cache[256]; /*Summary: Good idea, the implementation looks correct, I stumbled over some of the names, bitsizeof() could be used.
Thanks for the review. I'll squash up the following for v2 while awaiting feedback for the rest of the series:
diff --git a/object-file.c b/object-file.c
index d33b84c4a4..6c397fb4f1 100644
--- a/object-file.c
+++ b/object-file.c@@ -2463,16 +2463,17 @@ struct oidtree *odb_loose_cache(struct object_directory *odb, { int subdir_nr = oid->hash[0]; struct strbuf buf = STRBUF_INIT; - size_t BM_SIZE = sizeof(odb->loose_objects_subdir_seen[0]) * CHAR_BIT; + size_t word_bits = bitsizeof(odb->loose_objects_subdir_seen[0]); + size_t word_index = subdir_nr / word_bits; + size_t mask = 1 << (subdir_nr % word_bits); uint32_t *bitmap; - uint32_t bit = 1 << (subdir_nr % BM_SIZE); if (subdir_nr < 0 || - subdir_nr >= ARRAY_SIZE(odb->loose_objects_subdir_seen) * BM_SIZE) + subdir_nr >= bitsizeof(odb->loose_objects_subdir_seen)) BUG("subdir_nr out of range"); - bitmap = &odb->loose_objects_subdir_seen[subdir_nr / BM_SIZE]; - if (*bitmap & bit) + bitmap = &odb->loose_objects_subdir_seen[word_index]; + if (*bitmap & mask) return &odb->loose_objects_cache; strbuf_addstr(&buf, odb->path);
@@ -2480,7 +2481,7 @@ struct oidtree *odb_loose_cache(struct object_directory *odb, append_loose_object, NULL, NULL, &odb->loose_objects_cache); - *bitmap |= bit; + *bitmap |= mask; strbuf_release(&buf); return &odb->loose_objects_cache; }