Subject: Re: [PATCH 4/4] t5326: test propagating hashcache values

From: Taylor Blau <[email-masked]>
Date: Tue, 14 Sep 2021 01:31:01 -0400

On Tue, Sep 14, 2021 at 01:27:31AM -0400, Jeff King wrote:
> On Tue, Sep 14, 2021 at 01:11:34AM -0400, Taylor Blau wrote:
>
> > Some experiments to back that up: I instrumented the existing p5326 by
> > replacing anything like "pack-objects ... --stdout >/dev/null" with
> > "pack-objects ... --stdout >pack.tmp" and then added test_size's to
> > measure the size of each pack.
> >
> > On the tip of this branch, the results are:
> >
> > 		Test                              origin/tb/multi-pack-bitmaps   HEAD
> > 		----------------------------------------------------------------------------
> > 		5326.5: simulated clone size                 3.3G                 3.3G +0.0%
> > 		5326.7: simulated fetch size                10.5M                10.5M -0.2%
> > 		5326.21: clone (partial bitmap)              3.3G                 3.3G +0.0%
>
> I wouldn't expect a change in the clone size. We're already including
> all the objects from the single pack, so we won't even look for new
> deltas.
>
> In my run, I did see a small improvement in the fetch size (though my
> size both before and after was larger than yours). This is going to
> depend on the exact set of deltas we have (which in turn depends on how
> your repo happens to have been packed before the script even starts) and
> which ones the client actually wants (which may depend on the exact tip
> of your repo).

Yes, I agree with all of that. I am still interested in trying to figure
out why the resulting clone size seems to go up (independent of the
changes here). I'm bisecting it, but it's slow, since every step
requires you to repack the kernel.

> Presumably you also saw a decrease in the user CPU time of 5326.6 here.
> If not, you may have forgotten the extra patch to create the pack
> bitmap.

I did, but didn't bother to include the timings in the quoted part,
since I already shared them in [1].

I have a handful of new patches for an updated version of this series
which explains the extra patch you are talking about, too.

Thanks,
Taylor

[1]: https://lore.kernel.org/git/[email-masked]/