Re: [PATCH v2 6/7] p5326: generate pack bitmaps before writing the MIDX bitmap

On Tue, Sep 14, 2021 at 06:06:14PM -0400, Taylor Blau wrote:

To help test the performance of permuting the contents of the hash-cache
when generating a MIDX bitmap, we need a bitmap which has its hash-cache
populated.

And since multi-pack bitmaps don't add *new* values to the hash-cache,
we have to rely on a single-pack bitmap to generate those values for us.

Therefore, generate a pack bitmap before the MIDX one in order to ensure
that the MIDX bitmap has entries in its hash-cache.

Makes sense. This is a little more contrived of a setup than the
original, but an utterly realistic one. If you are using midx bitmaps,
you are probably interspersing them with occasional full pack bitmaps.

It might be interesting to also do:

  rm -f .git/objects/pack/pack-*.bitmap

after generating the midx bitmap. That would confirm the further timing
tests are using the midx bitmap, and not ever "cheating" by looking at
the pack one (having poked in this direction before, I know that this
all works, so it would be a future-proofing thing).

This sort-of existed before your series, but I think is a bit "worse"
now: we are timing both "repack" and "multi-pack-index" write together.
So:

  - the timing for the midx write that we are interested in timing will
    be diluted by the much-bigger full-repack

  - we'll actually do _three_ full repacks (the default
    GIT_PERF_REPEAT_COUNT for the "run" script), since it's inside a
    test_perf()

So:

  test_expect_success 'start with bitmapped pack' '
	git repack -adb
  '

  test_perf 'setup multi-pack index' '
	git multi-pack-index write --bitmap
  '

would run faster and give us more interesting timings. Possibly you'd
want to drop the midx and its bitmaps as part of that test_perf, too.
The first run will be using the pack bitmap, and the others will use the
midx. I doubt it makes much difference either way, though.

And of course if you want to take my earlier suggestion, then it's easy
to add:

  test_expect_success 'drop pack bitmap' '
	rm -f .git/objects/pack/pack-*.bitmap
  '

afterwards; you wouldn't want to do it inside the test_perf() call
because of the repeat-count.

-Peff