Subject: Re: [PATCH v2 20/20] pack-revindex.c: avoid direct revindex access in 'offset_to_pack_pos()'

From: Taylor Blau <[email-masked]>
Date: Thu, 14 Jan 2021 11:56:09 -0500

On Wed, Jan 13, 2021 at 10:42:29PM -0800, Junio C Hamano wrote:
> Taylor Blau <[email-masked]> 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.

I'm skeptical that it does (take that with a grain of salt, since I
haven't done any per-function tests with perf, only "how long does it
take to run 'git cat-file --batch-check=%(objectsize:disk)' and so on").

But even if it were to make a difference, it'll get dwarfed in the next
series by the time that we now _don't_ have to spend building and
sorting the reverse index in memory for each new process.

Thanks,
Taylor