Re: [PATCH v3 09/10] pack-revindex: ensure that on-disk reverse indexes are given precedence

From: Jeff King <peff@peff.net>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, dstolee@microsoft.com, gitster@pobox.com,
	jrnieder@gmail.com
Subject: Re: [PATCH v3 09/10] pack-revindex: ensure that on-disk reverse indexes are given precedence
Date: Thu, 28 Jan 2021 19:53:34 -0500	[thread overview]
Message-ID: <YBNcjmn5Jm/EmHwS@coredump.intra.peff.net> (raw)
In-Reply-To: <a66d2f9f7c20eeb813656e66b3ad9d42f2eecf34.1611617820.git.me@ttaylorr.com>

On Mon, Jan 25, 2021 at 06:37:46PM -0500, Taylor Blau wrote:

> When an on-disk reverse index exists, there is no need to generate one
> in memory. In fact, doing so can be slow, and require large amounts of
> the heap.
> 
> Let's make sure that we treat the on-disk reverse index with precedence
> (i.e., that when it exists, we don't bother trying to generate an
> equivalent one in memory) by teaching Git how to conditionally die()
> when generating a reverse index in memory.
> 
> Then, add a test to ensure that when (a) an on-disk reverse index
> exists, and (b) when setting GIT_TEST_REV_INDEX_DIE_IN_MEMORY, that we
> do not die, implying that we read from the on-disk one.

I don't love this kind of hackery, as it will have to live forever if we
want to keep testing this feature. But I also don't know how else to
tell in the regular test suite that we are avoiding the slow path.

Would it be enough to instead add a t/perf test which checks the speed
of:

  echo HEAD | git cat-file --batch-check='%(objectsize:disk)'

? I hate relying on the perf suite, because it gets run way less often
(and requires a lot of squinting to interpret the results). But it
wouldn't require any extra code the binary, as it's observing the actual
user-visible thing we care about: speed.

(I guess we care as much or more about peak heap usage, but measuring
that is a whole other can of worms).

-Peff