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

[PATCH v2 18/26] midx: implement `midx_preferred_pack()`

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

When performing a binary search over the objects in a MIDX's bitmap (i.e. in pseudo-pack order), the reader reconstructs the pseudo-pack ordering using a combination of (a) the preferred pack, (b) the pack's lexical position in the MIDX based on pack names, and (c) the object offset within the pack.

In order to perform this binary search, the reader must know the identity of the preferred pack. This could be stored in the MIDX, but isn't for historical reasons, mostly because it can easily be inferred at read-time by looking at the object in the first bit position and finding out which pack it was selected from in the MIDX, like so:

    nth_midxed_pack_int_id(m, pack_pos_to_midx(m, 0));

In midx_to_pack_pos() which performs this binary search, we look up the identity of the preferred pack before each search. This is relatively quick, since it involves two table-driven lookups (one in the MIDX's revindex for `pack_pos_to_midx()`, and another in the MIDX's object table for `nth_midxed_pack_int_id()`).

But since the preferred pack does not change after the MIDX is written, it is safe to cache this value on the MIDX itself.

Write a helper to do just that, and rewrite all of the existing call-sites that care about the identity of the preferred pack in terms of this new helper.

This will prepare us for a subsequent patch where we will need to binary search through the MIDX's pseudo-pack order multiple times.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 midx.c                    | 20 ++++++++++++++++++++
 midx.h                    |  2 ++
 pack-bitmap.c             | 17 +++++++----------
 pack-bitmap.h             |  1 -
 pack-revindex.c           |  4 +++-
 t/helper/test-read-midx.c | 13 +++++--------
 6 files changed, 37 insertions(+), 20 deletions(-)
diff --git a/midx.c b/midx.c
index beaf0c0de4..85e1c2cd12 100644
--- a/midx.c
+++ b/midx.c
@@ -21,6 +21,7 @@
 #include "refs.h"
 #include "revision.h"
 #include "list-objects.h"
+#include "pack-revindex.h"
 
 #define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
 #define MIDX_VERSION 1
@@ -177,6 +178,8 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
 
 	m->num_packs = get_be32(m->data + MIDX_BYTE_NUM_PACKS);
 
+	m->preferred_pack_idx = -1;
+
 	cf = init_chunkfile(NULL);
 
 	if (read_table_of_contents(cf, m->data, midx_size,
@@ -460,6 +463,23 @@ int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name)
 	return midx_locate_pack(m, idx_or_pack_name, NULL);
 }
 
+int midx_preferred_pack(struct multi_pack_index *m, uint32_t *pack_int_id)
+{
+	if (m->preferred_pack_idx == -1) {
+		if (load_midx_revindex(m) < 0) {
+			m->preferred_pack_idx = -2;
+			return -1;
+		}
+
+		m->preferred_pack_idx =
+			nth_midxed_pack_int_id(m, pack_pos_to_midx(m, 0));
+	} else if (m->preferred_pack_idx == -2)
+		return -1; /* no revindex */
+
+	*pack_int_id = m->preferred_pack_idx;
+	return 0;
+}
+
 int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local)
 {
 	struct multi_pack_index *m;
diff --git a/midx.h b/midx.h
index 89c5aa637e..f87a8fff26 100644
--- a/midx.h
+++ b/midx.h
@@ -29,6 +29,7 @@ struct multi_pack_index {
 	unsigned char num_chunks;
 	uint32_t num_packs;
 	uint32_t num_objects;
+	int preferred_pack_idx;
 
 	int local;
 
@@ -74,6 +75,7 @@ int midx_contains_pack(struct multi_pack_index *m,
 		       const char *idx_or_pack_name);
 int midx_locate_pack(struct multi_pack_index *m, const char *idx_or_pack_name,
 		     uint32_t *pos);
+int midx_preferred_pack(struct multi_pack_index *m, uint32_t *pack_int_id);
 int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);
 
 /*
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 4d5a484678..1682f99596 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -338,7 +338,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 	struct stat st;
 	char *bitmap_name = midx_bitmap_filename(midx);
 	int fd = git_open(bitmap_name);
-	uint32_t i;
+	uint32_t i, preferred_pack;
 	struct packed_git *preferred;
 
 	if (fd < 0) {
@@ -393,7 +393,12 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 		}
 	}
 
-	preferred = bitmap_git->midx->packs[midx_preferred_pack(bitmap_git)];
+	if (midx_preferred_pack(bitmap_git->midx, &preferred_pack) < 0) {
+		warning(_("could not determine MIDX preferred pack"));
+		goto cleanup;
+	}
+
+	preferred = bitmap_git->midx->packs[preferred_pack];
 	if (!is_pack_valid(preferred)) {
 		warning(_("preferred pack (%s) is invalid"),
 			preferred->pack_name);
@@ -1926,14 +1931,6 @@ static int try_partial_reuse(struct bitmapped_pack *pack,
 	return 0;
 }
 
-uint32_t midx_preferred_pack(struct bitmap_index *bitmap_git)
-{
-	struct multi_pack_index *m = bitmap_git->midx;
-	if (!m)
-		BUG("midx_preferred_pack: requires non-empty MIDX");
-	return nth_midxed_pack_int_id(m, pack_pos_to_midx(bitmap_git->midx, 0));
-}
-
 static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git,
 						 struct bitmapped_pack *pack,
 						 struct bitmap *reuse)
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 7a12a2ce81..179b343912 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -77,7 +77,6 @@ int test_bitmap_hashes(struct repository *r);
 
 struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 					 int filter_provided_objects);
-uint32_t midx_preferred_pack(struct bitmap_index *bitmap_git);
 void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 					struct bitmapped_pack **packs_out,
 					size_t *packs_nr_out,
diff --git a/pack-revindex.c b/pack-revindex.c
index acf1dd9786..7dc6c776d5 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -542,7 +542,9 @@ int midx_to_pack_pos(struct multi_pack_index *m, uint32_t at, uint32_t *pos)
 	 * implicitly is preferred (and includes all its objects, since ties are
 	 * broken first by pack identifier).
 	 */
-	key.preferred_pack = nth_midxed_pack_int_id(m, pack_pos_to_midx(m, 0));
+	if (midx_preferred_pack(key.midx, &key.preferred_pack) < 0)
+		return error(_("could not determine preferred pack"));
+
 
 	found = bsearch(&key, m->revindex_data, m->num_objects,
 			sizeof(*m->revindex_data), midx_pack_order_cmp);
diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
index e48557aba1..4acae41bb9 100644
--- a/t/helper/test-read-midx.c
+++ b/t/helper/test-read-midx.c
@@ -6,6 +6,7 @@
 #include "pack-bitmap.h"
 #include "packfile.h"
 #include "setup.h"
+#include "gettext.h"
 
 static int read_midx_file(const char *object_dir, int show_objects)
 {
@@ -79,7 +80,7 @@ static int read_midx_checksum(const char *object_dir)
 static int read_midx_preferred_pack(const char *object_dir)
 {
 	struct multi_pack_index *midx = NULL;
-	struct bitmap_index *bitmap = NULL;
+	uint32_t preferred_pack;
 
 	setup_git_directory();
 
@@ -87,16 +88,12 @@ static int read_midx_preferred_pack(const char *object_dir)
 	if (!midx)
 		return 1;
 
-	bitmap = prepare_bitmap_git(the_repository);
-	if (!bitmap)
-		return 1;
-	if (!bitmap_is_midx(bitmap)) {
-		free_bitmap_index(bitmap);
+	if (midx_preferred_pack(midx, &preferred_pack) < 0) {
+		warning(_("could not determine MIDX preferred pack"));
 		return 1;
 	}
 
-	printf("%s\n", midx->pack_names[midx_preferred_pack(bitmap)]);
-	free_bitmap_index(bitmap);
+	printf("%s\n", midx->pack_names[preferred_pack]);
 	return 0;
 }
 
-- 
2.43.0.102.ga31d690331.dirty
Previous: Taylor BlauNext: Taylor Blau
Message 93 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.