Re: [PATCH v2 20/20] pack-revindex.c: avoid direct revindex access in 'offset_to_pack_pos()'
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 14, 2021, 06:42 UTC
- Message-ID
- <xmqqmtxcyq7e.fsf@gitster.c.googlers.com>
- In-Reply-To
- <8400ff6c9615b4c999b198c46b2e673ec0f2b14f.1610576604.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> To prepare for on-disk reverse indexes, remove a spot in > 'offset_to_pack_pos()' that looks at the 'revindex' array in 'struct > packed_git'.
Hmph, I somehow would have expected that this clean-up would be done before step [18/20], but that does not matter in the end. The end result looks fairly clean.
I wonder if the call overhead to pack_pos_to_offset(), relative to the direct indexing of an in-core array revindex[] followed by an access to a member .offset that we used to do, makes a measurable difference in this tight loop, though.
Show 30 quoted lines
> diff --git a/pack-revindex.c b/pack-revindex.c
> index a508d5f0a4..5e69bc7372 100644
> --- a/pack-revindex.c
> +++ b/pack-revindex.c
> @@ -177,21 +177,21 @@ int load_pack_revindex(struct packed_git *p)
> int offset_to_pack_pos(struct packed_git *p, off_t ofs, uint32_t *pos)
> {
> unsigned lo, hi;
> - const struct revindex_entry *revindex;
>
> if (load_pack_revindex(p) < 0)
> return -1;
>
> lo = 0;
> hi = p->num_objects + 1;
> - revindex = p->revindex;
>
> do {
> const unsigned mi = lo + (hi - lo) / 2;
> - if (revindex[mi].offset == ofs) {
> + off_t got = pack_pos_to_offset(p, mi);
> +
> + if (got == ofs) {
> *pos = mi;
> return 0;
> - } else if (ofs < revindex[mi].offset)
> + } else if (ofs < got)
> hi = mi;
> else
> lo = mi + 1;