From 626fd982a3ead1b0aae26c242fbabaaaaf4b6062 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Thu, 22 Mar 2018 13:40:08 -0400 Subject: [PATCH 1/4] sha1_name: convert struct min_abbrev_data to object_id This structure is only written to in one place, where we already have a struct object_id. Convert the struct to use a struct object_id instead. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- sha1_name.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 39e911c8ba..16e0003396 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -480,7 +480,7 @@ struct min_abbrev_data { unsigned int init_len; unsigned int cur_len; char *hex; - const unsigned char *hash; + const struct object_id *oid; }; static inline char get_hex_char_from_oid(const struct object_id *oid, @@ -526,7 +526,7 @@ static void find_abbrev_len_for_pack(struct packed_git *p, int cmp; current = nth_packed_object_sha1(p, mid); - cmp = hashcmp(mad->hash, current); + cmp = hashcmp(mad->oid->hash, current); if (!cmp) { match = 1; first = mid; @@ -603,7 +603,7 @@ int find_unique_abbrev_r(char *hex, const struct object_id *oid, int len) mad.init_len = len; mad.cur_len = len; mad.hex = hex; - mad.hash = oid->hash; + mad.oid = oid; find_abbrev_len_packed(&mad); From 3d475f46a8225a9cc549ac5cb2c51d6ecd9e0f7c Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 22 Mar 2018 13:40:09 -0400 Subject: [PATCH 2/4] packfile: define and use bsearch_pack() The method bsearch_hash() generalizes binary searches using a fanout table. The only consumer is currently find_pack_entry_one(). It requires a bit of pointer arithmetic to align the fanout table and the lookup table depending on the pack-index version. Extract the pack-index pointer arithmetic to a new method, bsearch_pack(), so this can be re-used in other code paths. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- packfile.c | 42 ++++++++++++++++++++++++++---------------- packfile.h | 8 ++++++++ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/packfile.c b/packfile.c index f26395ecab..69d3afda6c 100644 --- a/packfile.c +++ b/packfile.c @@ -1654,6 +1654,29 @@ out: return data; } +int bsearch_pack(const struct object_id *oid, const struct packed_git *p, uint32_t *result) +{ + const unsigned char *index_fanout = p->index_data; + const unsigned char *index_lookup; + int index_lookup_width; + + if (!index_fanout) + BUG("bsearch_pack called without a valid pack-index"); + + index_lookup = index_fanout + 4 * 256; + if (p->index_version == 1) { + index_lookup_width = 24; + index_lookup += 4; + } else { + index_lookup_width = 20; + index_fanout += 8; + index_lookup += 8; + } + + return bsearch_hash(oid->hash, (const uint32_t*)index_fanout, + index_lookup, index_lookup_width, result); +} + const unsigned char *nth_packed_object_sha1(struct packed_git *p, uint32_t n) { @@ -1720,30 +1743,17 @@ off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n) off_t find_pack_entry_one(const unsigned char *sha1, struct packed_git *p) { - const uint32_t *level1_ofs = p->index_data; const unsigned char *index = p->index_data; - unsigned stride; + struct object_id oid; uint32_t result; if (!index) { if (open_pack_index(p)) return 0; - level1_ofs = p->index_data; - index = p->index_data; - } - if (p->index_version > 1) { - level1_ofs += 2; - index += 8; - } - index += 4 * 256; - if (p->index_version > 1) { - stride = 20; - } else { - stride = 24; - index += 4; } - if (bsearch_hash(sha1, level1_ofs, index, stride, &result)) + hashcpy(oid.hash, sha1); + if (bsearch_pack(&oid, p, &result)) return nth_packed_object_offset(p, result); return 0; } diff --git a/packfile.h b/packfile.h index a7fca598d6..ec08cb2bb0 100644 --- a/packfile.h +++ b/packfile.h @@ -78,6 +78,14 @@ extern struct packed_git *add_packed_git(const char *path, size_t path_len, int */ extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr); +/* + * Perform binary search on a pack-index for a given oid. Packfile is expected to + * have a valid pack-index. + * + * See 'bsearch_hash' for more information. + */ +int bsearch_pack(const struct object_id *oid, const struct packed_git *p, uint32_t *result); + /* * Return the SHA-1 of the nth object within the specified packfile. * Open the index if it is not already open. The return value points From 0aaf05b3bdc1ce48bed9f8dcba54153a2b7cdc09 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 22 Mar 2018 13:40:10 -0400 Subject: [PATCH 3/4] sha1_name: use bsearch_pack() for abbreviations When computing abbreviation lengths for an object ID against a single packfile, the method find_abbrev_len_for_pack() currently implements binary search. This is one of several implementations. One issue with this implementation is that it ignores the fanout table in the pack- index. Translate this binary search to use the existing bsearch_pack() method that correctly uses a fanout table. Due to the use of the fanout table, the abbreviation computation is slightly faster than before. For a fully-repacked copy of the Linux repo, the following 'git log' commands improved: * git log --oneline --parents --raw Before: 59.2s After: 56.9s Rel %: -3.8% * git log --oneline --parents Before: 6.48s After: 5.91s Rel %: -8.9% Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- sha1_name.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 16e0003396..24894b3dbe 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -512,32 +512,16 @@ static void find_abbrev_len_for_pack(struct packed_git *p, struct min_abbrev_data *mad) { int match = 0; - uint32_t num, last, first = 0; + uint32_t num, first = 0; struct object_id oid; + const struct object_id *mad_oid; if (open_pack_index(p) || !p->num_objects) return; num = p->num_objects; - last = num; - while (first < last) { - uint32_t mid = first + (last - first) / 2; - const unsigned char *current; - int cmp; - - current = nth_packed_object_sha1(p, mid); - cmp = hashcmp(mad->oid->hash, current); - if (!cmp) { - match = 1; - first = mid; - break; - } - if (cmp > 0) { - first = mid + 1; - continue; - } - last = mid; - } + mad_oid = mad->oid; + match = bsearch_pack(mad_oid, p, &first); /* * first is now the position in the packfile where we would insert From 902f5a2119089472c905407cb0a9b3886d032ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 24 Mar 2018 17:41:08 +0100 Subject: [PATCH 4/4] sha1_name: use bsearch_pack() in unique_in_pack() Replace the custom binary search in unique_in_pack() with a call to bsearch_pack(). This reduces code duplication and makes use of the fan-out table of packs. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- sha1_name.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 24894b3dbe..0185c6081a 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -150,31 +150,14 @@ static int match_sha(unsigned len, const unsigned char *a, const unsigned char * static void unique_in_pack(struct packed_git *p, struct disambiguate_state *ds) { - uint32_t num, last, i, first = 0; + uint32_t num, i, first = 0; const struct object_id *current = NULL; if (open_pack_index(p) || !p->num_objects) return; num = p->num_objects; - last = num; - while (first < last) { - uint32_t mid = first + (last - first) / 2; - const unsigned char *current; - int cmp; - - current = nth_packed_object_sha1(p, mid); - cmp = hashcmp(ds->bin_pfx.hash, current); - if (!cmp) { - first = mid; - break; - } - if (cmp > 0) { - first = mid+1; - continue; - } - last = mid; - } + bsearch_pack(&ds->bin_pfx, p, &first); /* * At this point, "first" is the location of the lowest object