git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 25/26] pack-bitmap: enable reuse from all bitmapped packs

From
Taylor Blau <me@ttaylorr.com>
Date
Dec 14, 2023, 22:24 UTC
Message-ID
<7002cf08fe301f1de28137b798fab3c8c32337fa.1702592604.git.me@ttaylorr.com>
In-Reply-To
<cover.1702592603.git.me@ttaylorr.com>

Now that both the pack-bitmap and pack-objects code are prepared to handle marking and using objects from multiple bitmapped packs for verbatim reuse, allow marking objects from all bitmapped packs as eligible for reuse.

Within the `reuse_partial_packfile_from_bitmap()` function, we no longer only mark the pack whose first object is at bit position zero for reuse, and instead mark any pack contained in the MIDX as a reuse candidate.

Provide a handful of test cases in a new script (t5332) exercising interesting behavior for multi-pack reuse to ensure that we performed all of the previous steps correctly.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/config/pack.txt |  16 ++-
 builtin/pack-objects.c        |   6 +-
 pack-bitmap.c                 |  34 ++++--
 pack-bitmap.h                 |   3 +-
 t/t5332-multi-pack-reuse.sh   | 203 ++++++++++++++++++++++++++++++++++
 5 files changed, 245 insertions(+), 17 deletions(-)
 create mode 100755 t/t5332-multi-pack-reuse.sh
diff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt
index fe100d0fb7..9c630863e6 100644
--- a/Documentation/config/pack.txt
+++ b/Documentation/config/pack.txt
@@ -28,11 +28,17 @@ all existing objects. You can force recompression by passing the -F option
 to linkgit:git-repack[1].
 
 pack.allowPackReuse::
-	When true or "single", and when reachability bitmaps are enabled,
-	pack-objects will try to send parts of the bitmapped packfile
-	verbatim. This can reduce memory and CPU usage to serve fetches,
-	but might result in sending a slightly larger pack. Defaults to
-	true.
+	When true or "single", and when reachability bitmaps are
+	enabled, pack-objects will try to send parts of the bitmapped
+	packfile verbatim. When "multi", and when a multi-pack
+	reachability bitmap is available, pack-objects will try to send
+	parts of all packs in the MIDX.
++
+	If only a single pack bitmap is available, and
+	`pack.allowPackReuse` is set to "multi", reuse parts of just the
+	bitmapped packfile. This can reduce memory and CPU usage to
+	serve fetches, but might result in sending a slightly larger
+	pack. Defaults to true.
 
 pack.island::
 	An extended regular expression configuring a set of delta
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 684698f679..5d3c42035b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -232,6 +232,7 @@ static int use_bitmap_index = -1;
 static enum {
 	NO_PACK_REUSE = 0,
 	SINGLE_PACK_REUSE,
+	MULTI_PACK_REUSE,
 } allow_pack_reuse = SINGLE_PACK_REUSE;
 static enum {
 	WRITE_BITMAP_FALSE = 0,
@@ -3251,6 +3252,8 @@ static int git_pack_config(const char *k, const char *v,
 		if (res < 0) {
 			if (!strcasecmp(v, "single"))
 				allow_pack_reuse = SINGLE_PACK_REUSE;
+			else if (!strcasecmp(v, "multi"))
+				allow_pack_reuse = MULTI_PACK_REUSE;
 			else
 				die(_("invalid pack.allowPackReuse value: '%s'"), v);
 		} else if (res) {
@@ -4029,7 +4032,8 @@ static int get_object_list_from_bitmap(struct rev_info *revs)
 		reuse_partial_packfile_from_bitmap(bitmap_git,
 						   &reuse_packfiles,
 						   &reuse_packfiles_nr,
-						   &reuse_packfile_bitmap);
+						   &reuse_packfile_bitmap,
+						   allow_pack_reuse == MULTI_PACK_REUSE);
 
 	if (reuse_packfiles) {
 		reuse_packfile_objects = bitmap_popcount(reuse_packfile_bitmap);
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 242a5908f7..229a11fb00 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2040,7 +2040,8 @@ static int bitmapped_pack_cmp(const void *va, const void *vb)
 void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 					struct bitmapped_pack **packs_out,
 					size_t *packs_nr_out,
-					struct bitmap **reuse_out)
+					struct bitmap **reuse_out,
+					int multi_pack_reuse)
 {
 	struct repository *r = the_repository;
 	struct bitmapped_pack *packs = NULL;
@@ -2064,15 +2065,30 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 				free(packs);
 				return;
 			}
+
 			if (!pack.bitmap_nr)
-				continue; /* no objects from this pack */
-			if (pack.bitmap_pos)
-				continue; /* not preferred pack */
+				continue;
+
+			if (!multi_pack_reuse && pack.bitmap_pos) {
+				/*
+				 * If we're only reusing a single pack, skip
+				 * over any packs which are not positioned at
+				 * the beginning of the MIDX bitmap.
+				 *
+				 * This is consistent with the existing
+				 * single-pack reuse behavior, which only reuses
+				 * parts of the MIDX's preferred pack.
+				 */
+				continue;
+			}
 
 			ALLOC_GROW(packs, packs_nr + 1, packs_alloc);
 			memcpy(&packs[packs_nr++], &pack, sizeof(pack));
 
 			objects_nr += pack.p->num_objects;
+
+			if (!multi_pack_reuse)
+				break;
 		}
 
 		QSORT(packs, packs_nr, bitmapped_pack_cmp);
@@ -2080,10 +2096,10 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 		ALLOC_GROW(packs, packs_nr + 1, packs_alloc);
 
 		packs[packs_nr].p = bitmap_git->pack;
-		packs[packs_nr].bitmap_pos = 0;
 		packs[packs_nr].bitmap_nr = bitmap_git->pack->num_objects;
+		packs[packs_nr].bitmap_pos = 0;
 
-		objects_nr = packs[packs_nr++].p->num_objects;
+		objects_nr = packs[packs_nr++].bitmap_nr;
 	}
 
 	word_alloc = objects_nr / BITS_IN_EWORD;
@@ -2091,10 +2107,8 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 		word_alloc++;
 	reuse = bitmap_word_alloc(word_alloc);
 
-	if (packs_nr != 1)
-		BUG("pack reuse not yet implemented for multiple packs");
-
-	reuse_partial_packfile_from_bitmap_1(bitmap_git, packs, reuse);
+	for (i = 0; i < packs_nr; i++)
+		reuse_partial_packfile_from_bitmap_1(bitmap_git, &packs[i], reuse);
 
 	if (bitmap_is_empty(reuse)) {
 		free(packs);
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 179b343912..c7dea13217 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -80,7 +80,8 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 					struct bitmapped_pack **packs_out,
 					size_t *packs_nr_out,
-					struct bitmap **reuse_out);
+					struct bitmap **reuse_out,
+					int multi_pack_reuse);
 int rebuild_existing_bitmaps(struct bitmap_index *, struct packing_data *mapping,
 			     kh_oid_map_t *reused_bitmaps, int show_progress);
 void free_bitmap_index(struct bitmap_index *);
diff --git a/t/t5332-multi-pack-reuse.sh b/t/t5332-multi-pack-reuse.sh
new file mode 100755
index 0000000000..2ba788b042
--- /dev/null
+++ b/t/t5332-multi-pack-reuse.sh
@@ -0,0 +1,203 @@
+#!/bin/sh
+
+test_description='pack-objects multi-pack reuse'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-bitmap.sh
+
+objdir=.git/objects
+packdir=$objdir/pack
+
+test_pack_reused () {
+	test_trace2_data pack-objects pack-reused "$1"
+}
+
+test_packs_reused () {
+	test_trace2_data pack-objects packs-reused "$1"
+}
+
+
+# pack_position <object> </path/to/pack.idx
+pack_position () {
+	git show-index >objects &&
+	grep "$1" objects | cut -d" " -f1
+}
+
+test_expect_success 'preferred pack is reused for single-pack reuse' '
+	test_config pack.allowPackReuse single &&
+
+	for i in A B
+	do
+		test_commit "$i" &&
+		git repack -d || return 1
+	done &&
+
+	git multi-pack-index write --bitmap &&
+
+	: >trace2.txt &&
+	GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+		git pack-objects --stdout --revs --all >/dev/null &&
+
+	test_pack_reused 3 <trace2.txt &&
+	test_packs_reused 1 <trace2.txt
+'
+
+test_expect_success 'enable multi-pack reuse' '
+	git config pack.allowPackReuse multi
+'
+
+test_expect_success 'reuse all objects from subset of bitmapped packs' '
+	test_commit C &&
+	git repack -d &&
+
+	git multi-pack-index write --bitmap &&
+
+	cat >in <<-EOF &&
+	$(git rev-parse C)
+	^$(git rev-parse A)
+	EOF
+
+	: >trace2.txt &&
+	GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+		git pack-objects --stdout --revs <in >/dev/null &&
+
+	test_pack_reused 6 <trace2.txt &&
+	test_packs_reused 2 <trace2.txt
+'
+
+test_expect_success 'reuse all objects from all packs' '
+	: >trace2.txt &&
+	GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+		git pack-objects --stdout --revs --all >/dev/null &&
+
+	test_pack_reused 9 <trace2.txt &&
+	test_packs_reused 3 <trace2.txt
+'
+
+test_expect_success 'reuse objects from first pack with middle gap' '
+	for i in D E F
+	do
+		test_commit "$i" || return 1
+	done &&
+
+	# Set "pack.window" to zero to ensure that we do not create any
+	# deltas, which could alter the amount of pack reuse we perform
+	# (if, for e.g., we are not sending one or more bases).
+	D="$(git -c pack.window=0 pack-objects --all --unpacked $packdir/pack)" &&
+
+	d_pos="$(pack_position $(git rev-parse D) <$packdir/pack-$D.idx)" &&
+	e_pos="$(pack_position $(git rev-parse E) <$packdir/pack-$D.idx)" &&
+	f_pos="$(pack_position $(git rev-parse F) <$packdir/pack-$D.idx)" &&
+
+	# commits F, E, and D, should appear in that order at the
+	# beginning of the pack
+	test $f_pos -lt $e_pos &&
+	test $e_pos -lt $d_pos &&
+
+	# Ensure that the pack we are constructing sorts ahead of any
+	# other packs in lexical/bitmap order by choosing it as the
+	# preferred pack.
+	git multi-pack-index write --bitmap --preferred-pack="pack-$D.idx" &&
+
+	cat >in <<-EOF &&
+	$(git rev-parse E)
+	^$(git rev-parse D)
+	EOF
+
+	: >trace2.txt &&
+	GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+		git pack-objects --stdout --delta-base-offset --revs <in >/dev/null &&
+
+	test_pack_reused 3 <trace2.txt &&
+	test_packs_reused 1 <trace2.txt
+'
+
+test_expect_success 'reuse objects from middle pack with middle gap' '
+	rm -fr $packdir/multi-pack-index* &&
+
+	# Ensure that the pack we are constructing sort into any
+	# position *but* the first one, by choosing a different pack as
+	# the preferred one.
+	git multi-pack-index write --bitmap --preferred-pack="pack-$A.idx" &&
+
+	cat >in <<-EOF &&
+	$(git rev-parse E)
+	^$(git rev-parse D)
+	EOF
+
+	: >trace2.txt &&
+	GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+		git pack-objects --stdout --delta-base-offset --revs <in >/dev/null &&
+
+	test_pack_reused 3 <trace2.txt &&
+	test_packs_reused 1 <trace2.txt
+'
+
+test_expect_success 'omit delta with uninteresting base (same pack)' '
+	git repack -adk &&
+
+	test_seq 32 >f &&
+	git add f &&
+	test_tick &&
+	git commit -m "delta" &&
+	delta="$(git rev-parse HEAD)" &&
+
+	test_seq 64 >f &&
+	test_tick &&
+	git commit -a -m "base" &&
+	base="$(git rev-parse HEAD)" &&
+
+	test_commit other &&
+
+	git repack -d &&
+
+	have_delta "$(git rev-parse $delta:f)" "$(git rev-parse $base:f)" &&
+
+	git multi-pack-index write --bitmap &&
+
+	cat >in <<-EOF &&
+	$(git rev-parse other)
+	^$base
+	EOF
+
+	: >trace2.txt &&
+	GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+		git pack-objects --stdout --delta-base-offset --revs <in >/dev/null &&
+
+	# We can only reuse the 3 objects corresponding to "other" from
+	# the latest pack.
+	#
+	# This is because even though we want "delta", we do not want
+	# "base", meaning that we have to inflate the delta/base-pair
+	# corresponding to the blob in commit "delta", which bypasses
+	# the pack-reuse mechanism.
+	#
+	# The remaining objects from the other pack are similarly not
+	# reused because their objects are on the uninteresting side of
+	# the query.
+	test_pack_reused 3 <trace2.txt &&
+	test_packs_reused 1 <trace2.txt
+'
+
+test_expect_success 'omit delta from uninteresting base (cross pack)' '
+	cat >in <<-EOF &&
+	$(git rev-parse $base)
+	^$(git rev-parse $delta)
+	EOF
+
+	P="$(git pack-objects --revs $packdir/pack <in)" &&
+
+	git multi-pack-index write --bitmap --preferred-pack="pack-$P.idx" &&
+
+	: >trace2.txt &&
+	GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+		git pack-objects --stdout --delta-base-offset --all >/dev/null &&
+
+	packs_nr="$(find $packdir -type f -name "pack-*.pack" | wc -l)" &&
+	objects_nr="$(git rev-list --count --all --objects)" &&
+
+	test_pack_reused $(($objects_nr - 1)) <trace2.txt &&
+	test_packs_reused $packs_nr <trace2.txt
+'
+
+test_done

-- 
2.43.0.102.ga31d690331.dirty
Previous: Taylor BlauNext: Taylor Blau
Message 100 of 107 in “pack-objects: multi-pack verbatim reuse”
  1. 00/24 pack-objects: multi-pack verbatim reuseTaylor Blau, Nov 28, 2023
  2. 01/24 pack-objects: free packing_data in more placesTaylor Blau, Nov 28, 2023
  3. Patrick SteinhardtNov 30, 2023
  4. Taylor BlauNov 30, 2023
  5. 02/24 pack-bitmap-write: deep-clear the `bb_commit` slabTaylor Blau, Nov 28, 2023
  6. Patrick SteinhardtNov 30, 2023
  7. Taylor BlauNov 30, 2023
  8. Jeff KingDec 12, 2023
  9. Taylor BlauDec 12, 2023
  10. 03/24 pack-bitmap: plug leak in find_objects()Taylor Blau, Nov 28, 2023
  11. Jeff KingDec 12, 2023
  12. 04/24 midx: factor out `fill_pack_info()`Taylor Blau, Nov 28, 2023
  13. Patrick SteinhardtNov 30, 2023
  14. Taylor BlauNov 30, 2023
  15. 05/24 midx: implement `DISP` chunkTaylor Blau, Nov 28, 2023
  16. Patrick SteinhardtNov 30, 2023
  17. Taylor BlauNov 30, 2023
  18. Junio C HamanoDec 3, 2023
  19. Taylor BlauDec 5, 2023
  20. Junio C HamanoDec 9, 2023
  21. Taylor BlauDec 9, 2023
  22. Jeff KingDec 12, 2023
  23. Taylor BlauDec 13, 2023
  24. Junio C HamanoDec 13, 2023
  25. 06/24 midx: implement `midx_locate_pack()`Taylor Blau, Nov 28, 2023
  26. 07/24 midx: implement `--retain-disjoint` modeTaylor Blau, Nov 28, 2023
  27. Patrick SteinhardtNov 30, 2023
  28. Taylor BlauNov 30, 2023
  29. Patrick SteinhardtDec 1, 2023
  30. 08/24 pack-objects: implement `--ignore-disjoint` modeTaylor Blau, Nov 28, 2023
  31. Patrick SteinhardtNov 30, 2023
  32. Taylor BlauNov 30, 2023
  33. Patrick SteinhardtDec 1, 2023
  34. Taylor BlauDec 1, 2023
  35. 09/24 repack: implement `--extend-disjoint` modeTaylor Blau, Nov 28, 2023
  36. Patrick SteinhardtDec 7, 2023
  37. Taylor BlauDec 7, 2023
  38. Patrick SteinhardtDec 8, 2023
  39. Taylor BlauDec 8, 2023
  40. Patrick SteinhardtDec 11, 2023
  41. Taylor BlauDec 11, 2023
  42. 10/24 pack-bitmap: pass `bitmapped_pack` struct to pack-reuse functionsTaylor Blau, Nov 28, 2023
  43. Patrick SteinhardtDec 7, 2023
  44. Taylor BlauDec 7, 2023
  45. Patrick SteinhardtDec 8, 2023
  46. 11/24 pack-bitmap: simplify `reuse_partial_packfile_from_bitmap()` signatureTaylor Blau, Nov 28, 2023
  47. Patrick SteinhardtDec 7, 2023
  48. Taylor BlauDec 7, 2023
  49. 12/24 pack-bitmap: return multiple packs via `reuse_partial_packfile_from_bitmap()`Taylor Blau, Nov 28, 2023
  50. 13/24 pack-objects: parameterize pack-reuse routines over a single packTaylor Blau, Nov 28, 2023
  51. 14/24 pack-objects: keep track of `pack_start` for each reuse packTaylor Blau, Nov 28, 2023
  52. Patrick SteinhardtDec 7, 2023
  53. Taylor BlauDec 7, 2023
  54. 15/24 pack-objects: pass `bitmapped_pack`'s to pack-reuse functionsTaylor Blau, Nov 28, 2023
  55. 16/24 pack-objects: prepare `write_reused_pack()` for multi-pack reuseTaylor Blau, Nov 28, 2023
  56. Patrick SteinhardtDec 7, 2023
  57. Taylor BlauDec 7, 2023
  58. 17/24 pack-objects: prepare `write_reused_pack_verbatim()` for multi-pack reuseTaylor Blau, Nov 28, 2023
  59. 18/24 pack-objects: include number of packs reused in outputTaylor Blau, Nov 28, 2023
  60. 19/24 pack-bitmap: prepare to mark objects from multiple packs for reuseTaylor Blau, Nov 28, 2023
  61. 20/24 pack-objects: add tracing for various packfile metricsTaylor Blau, Nov 28, 2023
  62. 21/24 t/test-lib-functions.sh: implement `test_trace2_data` helperTaylor Blau, Nov 28, 2023
  63. 22/24 pack-objects: allow setting `pack.allowPackReuse` to "single"Taylor Blau, Nov 28, 2023
  64. 23/24 pack-bitmap: reuse objects from all disjoint packsTaylor Blau, Nov 28, 2023
  65. 24/24 t/perf: add performance tests for multi-pack reuseTaylor Blau, Nov 28, 2023
  66. Patrick SteinhardtNov 30, 2023
  67. Taylor BlauNov 30, 2023
  68. Patrick SteinhardtDec 1, 2023
  69. Taylor BlauDec 1, 2023
  70. Patrick SteinhardtDec 4, 2023
  71. Jeff KingDec 12, 2023
  72. Taylor BlauDec 15, 2023
  73. Jeff KingDec 21, 2023
  74. Taylor BlauJan 4, 2024
  75. 00/26 pack-objects: multi-pack verbatim reuseTaylor Blau, Dec 14, 2023
  76. 01/26 pack-objects: free packing_data in more placesTaylor Blau, Dec 14, 2023
  77. 02/26 pack-bitmap-write: deep-clear the `bb_commit` slabTaylor Blau, Dec 14, 2023
  78. 03/26 pack-bitmap: plug leak in find_objects()Taylor Blau, Dec 14, 2023
  79. 04/26 midx: factor out `fill_pack_info()`Taylor Blau, Dec 14, 2023
  80. 05/26 midx: implement `BTMP` chunkTaylor Blau, Dec 14, 2023
  81. 06/26 midx: implement `midx_locate_pack()`Taylor Blau, Dec 14, 2023
  82. 07/26 pack-bitmap: pass `bitmapped_pack` struct to pack-reuse functionsTaylor Blau, Dec 14, 2023
  83. 08/26 ewah: implement `bitmap_is_empty()`Taylor Blau, Dec 14, 2023
  84. 09/26 pack-bitmap: simplify `reuse_partial_packfile_from_bitmap()` signatureTaylor Blau, Dec 14, 2023
  85. 10/26 pack-bitmap: return multiple packs via `reuse_partial_packfile_from_bitmap()`Taylor Blau, Dec 14, 2023
  86. 11/26 pack-objects: parameterize pack-reuse routines over a single packTaylor Blau, Dec 14, 2023
  87. 12/26 pack-objects: keep track of `pack_start` for each reuse packTaylor Blau, Dec 14, 2023
  88. 13/26 pack-objects: pass `bitmapped_pack`'s to pack-reuse functionsTaylor Blau, Dec 14, 2023
  89. 14/26 pack-objects: prepare `write_reused_pack()` for multi-pack reuseTaylor Blau, Dec 14, 2023
  90. 15/26 pack-objects: prepare `write_reused_pack_verbatim()` for multi-pack reuseTaylor Blau, Dec 14, 2023
  91. 16/26 pack-objects: include number of packs reused in outputTaylor Blau, Dec 14, 2023
  92. 17/26 git-compat-util.h: implement checked size_t to uint32_t conversionTaylor Blau, Dec 14, 2023
  93. 18/26 midx: implement `midx_preferred_pack()`Taylor Blau, Dec 14, 2023
  94. 19/26 pack-revindex: factor out `midx_key_to_pack_pos()` helperTaylor Blau, Dec 14, 2023
  95. 20/26 pack-revindex: implement `midx_pair_to_pack_pos()`Taylor Blau, Dec 14, 2023
  96. 21/26 pack-bitmap: prepare to mark objects from multiple packs for reuseTaylor Blau, Dec 14, 2023
  97. 22/26 pack-objects: add tracing for various packfile metricsTaylor Blau, Dec 14, 2023
  98. 23/26 t/test-lib-functions.sh: implement `test_trace2_data` helperTaylor Blau, Dec 14, 2023
  99. 24/26 pack-objects: allow setting `pack.allowPackReuse` to "single"Taylor Blau, Dec 14, 2023
  100. 25/26 pack-bitmap: enable reuse from all bitmapped packsTaylor Blau, Dec 14, 2023
  101. 26/26 t/perf: add performance tests for multi-pack reuseTaylor Blau, Dec 14, 2023
  102. Junio C HamanoDec 15, 2023
  103. Taylor BlauDec 15, 2023
  104. Junio C HamanoDec 15, 2023
  105. Taylor BlauDec 15, 2023
  106. Jeff KingDec 21, 2023
  107. Taylor BlauJan 4, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.