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

[PATCH v2 7/8] builtin/repack.c: make largest pack preferred

From
Taylor Blau <me@ttaylorr.com>
Date
Sep 15, 2021, 18:24 UTC
Message-ID
<68bc49d8aed438303e422530c74f2e970c7d66a4.1631730270.git.me@ttaylorr.com>
In-Reply-To
<cover.1631730270.git.me@ttaylorr.com>

When repacking into a geometric series and writing a multi-pack bitmap, it is beneficial to have the largest resulting pack be the preferred object source in the bitmap's MIDX, since selecting the large packs can lead to fewer broken delta chains and better compression.

Teach 'git repack' to identify this pack and pass it to the MIDX write machinery in order to mark it as preferred.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-repack.txt |  4 ++++
 builtin/repack.c             | 27 ++++++++++++++++++++++++++-
 pack-bitmap.c                |  2 +-
 pack-bitmap.h                |  1 +
 t/helper/test-read-midx.c    | 25 ++++++++++++++++++++++++-
 t/t7703-repack-geometric.sh  | 22 ++++++++++++++++++++++
 6 files changed, 78 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 0f2d235ca5..7183fb498f 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -190,6 +190,10 @@ this "roll-up", without respect to their reachability. This is subject
 to change in the future. This option (implying a drastically different
 repack mode) is not guaranteed to work with all other combinations of
 option to `git repack`.
++
+When writing a multi-pack bitmap, `git repack` selects the largest resulting
+pack as the preferred pack for object selection by the MIDX (see
+linkgit:git-multi-pack-index[1]).
 
 -m::
 --write-midx::
diff --git a/builtin/repack.c b/builtin/repack.c
index ca18d3dd2e..8c7bc4551e 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -422,6 +422,25 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor)
 	geometry->split = split;
 }
 
+static struct packed_git *get_largest_active_pack(struct pack_geometry *geometry)
+{
+	if (!geometry) {
+		/*
+		 * No geometry means either an all-into-one repack (in which
+		 * case there is only one pack left and it is the largest) or an
+		 * incremental one.
+		 *
+		 * If repacking incrementally, then we could check the size of
+		 * all packs to determine which should be preferred, but leave
+		 * this for later.
+		 */
+		return NULL;
+	}
+	if (geometry->split == geometry->pack_nr)
+		return NULL;
+	return geometry->pack[geometry->pack_nr - 1];
+}
+
 static void clear_pack_geometry(struct pack_geometry *geometry)
 {
 	if (!geometry)
@@ -467,10 +486,12 @@ static void midx_included_packs(struct string_list *include,
 }
 
 static int write_midx_included_packs(struct string_list *include,
+				     struct pack_geometry *geometry,
 				     int show_progress, int write_bitmaps)
 {
 	struct child_process cmd = CHILD_PROCESS_INIT;
 	struct string_list_item *item;
+	struct packed_git *largest = get_largest_active_pack(geometry);
 	FILE *in;
 	int ret;
 
@@ -488,6 +509,10 @@ static int write_midx_included_packs(struct string_list *include,
 	if (write_bitmaps)
 		strvec_push(&cmd.args, "--bitmap");
 
+	if (largest)
+		strvec_pushf(&cmd.args, "--preferred-pack=%s",
+			     pack_basename(largest));
+
 	ret = start_command(&cmd);
 	if (ret)
 		return ret;
@@ -773,7 +798,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 		midx_included_packs(&include, &existing_packs,
 				    &existing_kept_packs, &names, geometry);
 
-		ret = write_midx_included_packs(&include,
+		ret = write_midx_included_packs(&include, geometry,
 						show_progress, write_bitmaps > 0);
 
 		string_list_clear(&include, 0);
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 8504110a4d..67be9be9a6 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1418,7 +1418,7 @@ static int try_partial_reuse(struct packed_git *pack,
 	return 0;
 }
 
-static uint32_t midx_preferred_pack(struct bitmap_index *bitmap_git)
+uint32_t midx_preferred_pack(struct bitmap_index *bitmap_git)
 {
 	struct multi_pack_index *m = bitmap_git->midx;
 	if (!m)
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 469090bad2..7d407c5a4c 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -55,6 +55,7 @@ int test_bitmap_commits(struct repository *r);
 struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 					 struct list_objects_filter_options *filter,
 					 int filter_provided_objects);
+uint32_t midx_preferred_pack(struct bitmap_index *bitmap_git);
 int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
 				       struct packed_git **packfile,
 				       uint32_t *entries,
diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
index cb0d27049a..0038559129 100644
--- a/t/helper/test-read-midx.c
+++ b/t/helper/test-read-midx.c
@@ -3,6 +3,7 @@
 #include "midx.h"
 #include "repository.h"
 #include "object-store.h"
+#include "pack-bitmap.h"
 
 static int read_midx_file(const char *object_dir, int show_objects)
 {
@@ -72,14 +73,36 @@ static int read_midx_checksum(const char *object_dir)
 	return 0;
 }
 
+static int read_midx_preferred_pack(const char *object_dir)
+{
+	struct multi_pack_index *midx = NULL;
+	struct bitmap_index *bitmap = NULL;
+
+	setup_git_directory();
+
+	midx = load_multi_pack_index(object_dir, 1);
+	if (!midx)
+		return 1;
+
+	bitmap = prepare_bitmap_git(the_repository);
+	if (!(bitmap && bitmap_is_midx(bitmap)))
+		return 1;
+
+
+	printf("%s\n", midx->pack_names[midx_preferred_pack(bitmap)]);
+	return 0;
+}
+
 int cmd__read_midx(int argc, const char **argv)
 {
 	if (!(argc == 2 || argc == 3))
-		usage("read-midx [--show-objects|--checksum] <object-dir>");
+		usage("read-midx [--show-objects|--checksum|--preferred-pack] <object-dir>");
 
 	if (!strcmp(argv[1], "--show-objects"))
 		return read_midx_file(argv[2], 1);
 	else if (!strcmp(argv[1], "--checksum"))
 		return read_midx_checksum(argv[2]);
+	else if (!strcmp(argv[1], "--preferred-pack"))
+		return read_midx_preferred_pack(argv[2]);
 	return read_midx_file(argv[1], 0);
 }
diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh
index 5ccaa440e0..54360c9878 100755
--- a/t/t7703-repack-geometric.sh
+++ b/t/t7703-repack-geometric.sh
@@ -180,4 +180,26 @@ test_expect_success '--geometric ignores kept packs' '
 	)
 '
 
+test_expect_success '--geometric chooses largest MIDX preferred pack' '
+	git init geometric &&
+	test_when_finished "rm -fr geometric" &&
+	(
+		cd geometric &&
+
+		# These packs already form a geometric progression.
+		test_commit_bulk --start=1 1 && # 3 objects
+		test_commit_bulk --start=2 2 && # 6 objects
+		ls $objdir/pack/pack-*.idx >before &&
+		test_commit_bulk --start=4 4 && # 12 objects
+		ls $objdir/pack/pack-*.idx >after &&
+
+		git repack --geometric 2 -dbm &&
+
+		comm -3 before after | xargs -n 1 basename >expect &&
+		test-tool read-midx --preferred-pack $objdir >actual &&
+
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.33.0.96.g73915697e6
Previous: Taylor BlauNext: Taylor Blau
Message 56 of 76 in “repack: introduce `--write-midx`”
  1. 0/8 repack: introduce `--write-midx`Taylor Blau, Sep 11, 2021
  2. 1/8 midx: expose 'write_midx_file_only()' publiclyTaylor Blau, Sep 11, 2021
  3. Junio C HamanoSep 11, 2021
  4. Taylor BlauSep 11, 2021
  5. Ævar Arnfjörð BjarmasonSep 11, 2021
  6. Taylor BlauSep 11, 2021
  7. 2/8 builtin/multi-pack-index.c: support --stdin-packs modeTaylor Blau, Sep 11, 2021
  8. Ævar Arnfjörð BjarmasonSep 11, 2021
  9. Taylor BlauSep 11, 2021
  10. Taylor BlauSep 11, 2021
  11. Eric SunshineSep 12, 2021
  12. Taylor BlauSep 12, 2021
  13. Ævar Arnfjörð BjarmasonSep 12, 2021
  14. Junio C HamanoSep 12, 2021
  15. Ævar Arnfjörð BjarmasonSep 12, 2021
  16. Jeff KingSep 14, 2021
  17. Taylor BlauSep 14, 2021
  18. Eric SunshineSep 15, 2021
  19. 4/8 builtin/repack.c: keep track of existing packs unconditionallyTaylor Blau, Sep 11, 2021
  20. 3/8 midx: preliminary support for `--refs-snapshot`Taylor Blau, Sep 11, 2021
  21. 5/8 builtin/repack.c: extract showing progress to a variableTaylor Blau, Sep 11, 2021
  22. 6/8 builtin/repack.c: support writing a MIDX while repackingTaylor Blau, Sep 11, 2021
  23. 7/8 builtin/repack.c: make largest pack preferredTaylor Blau, Sep 11, 2021
  24. Ævar Arnfjörð BjarmasonSep 11, 2021
  25. Taylor BlauSep 11, 2021
  26. 8/8 builtin/repack.c: pass `--refs-snapshot` when writing bitmapsTaylor Blau, Sep 11, 2021
  27. Ævar Arnfjörð BjarmasonSep 11, 2021
  28. Ævar Arnfjörð BjarmasonSep 11, 2021
  29. Taylor BlauSep 11, 2021
  30. Jeff KingSep 14, 2021
  31. Taylor BlauSep 14, 2021
  32. Ævar Arnfjörð BjarmasonSep 14, 2021
  33. Taylor BlauSep 15, 2021
  34. Taylor BlauSep 11, 2021
  35. 0/8 repack: introduce `--write-midx`Taylor Blau, Sep 15, 2021
  36. 1/8 midx: expose `write_midx_file_only()` publiclyTaylor Blau, Sep 15, 2021
  37. Jonathan TanSep 22, 2021
  38. Taylor BlauSep 23, 2021
  39. 2/8 builtin/multi-pack-index.c: support `--stdin-packs` modeTaylor Blau, Sep 15, 2021
  40. Josh SteadmonSep 22, 2021
  41. Taylor BlauSep 23, 2021
  42. Jonathan TanSep 22, 2021
  43. Taylor BlauSep 23, 2021
  44. 3/8 midx: preliminary support for `--refs-snapshot`Taylor Blau, Sep 15, 2021
  45. Josh SteadmonSep 22, 2021
  46. Taylor BlauSep 23, 2021
  47. Jonathan TanSep 22, 2021
  48. Taylor BlauSep 23, 2021
  49. 4/8 builtin/repack.c: keep track of existing packs unconditionallyTaylor Blau, Sep 15, 2021
  50. Jonathan TanSep 22, 2021
  51. Taylor BlauSep 23, 2021
  52. 5/8 builtin/repack.c: extract showing progress to a variableTaylor Blau, Sep 15, 2021
  53. 6/8 builtin/repack.c: support writing a MIDX while repackingTaylor Blau, Sep 15, 2021
  54. Jonathan TanSep 22, 2021
  55. Taylor BlauSep 23, 2021
  56. 7/8 builtin/repack.c: make largest pack preferredTaylor Blau, Sep 15, 2021
  57. 8/8 builtin/repack.c: pass `--refs-snapshot` when writing bitmapsTaylor Blau, Sep 15, 2021
  58. Jonathan TanSep 24, 2021
  59. Taylor BlauOct 1, 2021
  60. Junio C HamanoSep 15, 2021
  61. Junio C HamanoSep 15, 2021
  62. Taylor BlauSep 15, 2021
  63. Junio C HamanoSep 16, 2021
  64. 0/9 repack: introduce `--write-midx`Taylor Blau, Sep 29, 2021
  65. 1/9 midx: expose `write_midx_file_only()` publiclyTaylor Blau, Sep 29, 2021
  66. 2/9 builtin/multi-pack-index.c: support `--stdin-packs` modeTaylor Blau, Sep 29, 2021
  67. 3/9 midx: preliminary support for `--refs-snapshot`Taylor Blau, Sep 29, 2021
  68. 4/9 builtin/repack.c: keep track of existing packs unconditionallyTaylor Blau, Sep 29, 2021
  69. 5/9 builtin/repack.c: rename variables that deal with non-kept packsTaylor Blau, Sep 29, 2021
  70. 6/9 builtin/repack.c: extract showing progress to a variableTaylor Blau, Sep 29, 2021
  71. 7/9 builtin/repack.c: support writing a MIDX while repackingTaylor Blau, Sep 29, 2021
  72. 8/9 builtin/repack.c: make largest pack preferredTaylor Blau, Sep 29, 2021
  73. 9/9 builtin/repack.c: pass `--refs-snapshot` when writing bitmapsTaylor Blau, Sep 29, 2021
  74. Junio C HamanoSep 29, 2021
  75. Jonathan TanOct 1, 2021
  76. Taylor BlauOct 1, 2021

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.