From: Junio C Hamano <hidden> Date: 2021-09-14 02:05:41
Taylor Blau [off-list ref] writes:
Alas, there they are. They are basically no different than having the
name-hash for single pack bitmaps, it's just now we don't throw them
away when generating a MIDX bitmap from a state where the repository
already has a single-pack bitmap.
I actually wasn't expecting any CPU/time difference.
I hope that we are talking about the same name-hash, which is used
to sort the blobs so that when pack-objects try to find a good delta
base, the blobs from the same path will sit close to each other and
hopefully fit in the pack window.
The effect I was hoping to see by not discarding the information was
that we find better delta base hence smaller deltas in the resulting
packfiles.
Thanks.
From: Taylor Blau <hidden> Date: 2021-09-14 05:11:42
On Mon, Sep 13, 2021 at 07:05:32PM -0700, Junio C Hamano wrote:
Taylor Blau [off-list ref] writes:
quoted
Alas, there they are. They are basically no different than having the
name-hash for single pack bitmaps, it's just now we don't throw them
away when generating a MIDX bitmap from a state where the repository
already has a single-pack bitmap.
I actually wasn't expecting any CPU/time difference.
I think it is possible to see the CPU usage go down without affecting the
resulting pack size. See below for a more detailed analysis.
I hope that we are talking about the same name-hash, which is used
to sort the blobs so that when pack-objects try to find a good delta
base, the blobs from the same path will sit close to each other and
hopefully fit in the pack window.
Yes, of course.
The effect I was hoping to see by not discarding the information was
that we find better delta base hence smaller deltas in the resulting
packfiles.
I think it is possible to observe either a decrease in CPU or a decrease
in the resulting pack size.
In my experience having the name-hash filled in results in finding good
delta pairs much more quickly than without, but that in many
repositories the resulting pack size is basically the same. In other
words, the resulting pack is pretty similar whether you use the
name-hash or not, it just affects how quickly you get there.
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%
Looking at c171d3e677 (pack-bitmap: implement optional name_hash cache,
2013-10-22), I modified[1] that script to replace timing pack-objects with
counting the number of bytes it wrote.
Doing that shows that the name-hash doesn't make a substantial difference in
the resulting pack size (numbers on a recent-ish copy of the kernel):
Test c171d3e677d777c50231d8dea32ae691936da819^ c171d3e677d777c50231d8dea32ae691936da819
--------------------------------------------------------------------------------------------------------------
9999.3: simulated clone 3.2G 3.2G +0.0%
9999.4: simulated fetch 32 32 +0.0%
9999.6: partial bitmap 3.1G 3.1G +0.0%
(As a mostly-unrelated aside, I was curious why the pack size jumped from 3.2GB
to 3.3GB, but I can reproduce that jump even in p5310--the single pack bitmap
test--on the tip of my branch. So it does appear to be a regression which I'll
look into, but it's unrelated to this branch or MIDX bitmaps).
Thanks,
Taylor
[1]: https://gist.github.com/ttaylorr/6cfa3eb9fd012f81b833873d50f96f71
From: Jeff King <hidden> Date: 2021-09-14 05:23:23
On Mon, Sep 13, 2021 at 07:05:32PM -0700, Junio C Hamano wrote:
Taylor Blau [off-list ref] writes:
quoted
Alas, there they are. They are basically no different than having the
name-hash for single pack bitmaps, it's just now we don't throw them
away when generating a MIDX bitmap from a state where the repository
already has a single-pack bitmap.
I actually wasn't expecting any CPU/time difference.
I was, for the same reason we saw an improvement there in ae4f07fbcc
(pack-bitmap: implement optional name_hash cache, 2013-12-21): without a
name-hash, we try a bunch of fruitless deltas before we find a decent
one.
I hope that we are talking about the same name-hash, which is used
to sort the blobs so that when pack-objects try to find a good delta
base, the blobs from the same path will sit close to each other and
hopefully fit in the pack window.
Yes, exactly. We spend less time finding the good ones if the likely
candidates are close together. We may _also_ find better ones overall,
depending on the number of candidates and the window size.
The bitmap perf tests (neither p5310 nor its new midx cousin p5326)
don't check the output size.
The effect I was hoping to see by not discarding the information was
that we find better delta base hence smaller deltas in the resulting
packfiles.
then the results I get using linux.git are:
Test origin/tb/multi-pack-bitmaps origin/tb/midx-write-propagate-namehash
-------------------------------------------------------------------------------------------------
5326.4: simulated fetch 2.32(7.16+0.21) 2.00(3.79+0.18) -13.8%
5326.5: fetch size 16.7M 15.5M -7.1%
so you can see that we spent about half as much CPU (ignore the
wall-clock percentage; the interesting thing is the userspace time,
because my machine has 8 cores). But we also shaved off a bit from the
pack, so we really did manage to find better deltas, too.
I see that Taylor just posted a very similar response, and independently
did the exact same experiment I did. ;) I'll send this anyway, though,
as my particular run showed slightly different results.
-Peff
[1] The other thing you'd want (and I presume Taylor was using for his
earlier timings) is:
From: Jeff King <hidden> Date: 2021-09-14 05:27:33
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).
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.
-Peff
From: Taylor Blau <hidden> Date: 2021-09-14 05:31:05
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:
quoted
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/YT%2F3BuDa7KfUN%2F38@nand.local/