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

[PATCH 07/11] write_reused_pack_one(): translate bit positions directly

From
Taylor Blau <me@ttaylorr.com>
Date
Oct 9, 2024, 20:31 UTC
Message-ID
<94e5c96f6859479e0206d6d775eacf54b3639ee5.1728505840.git.me@ttaylorr.com>
In-Reply-To
<cover.1728505840.git.me@ttaylorr.com>

A future commit will want to deal with bit positions instead of pack positions from within builtin/pack-objects.c::write_reused_pack_one().

That function at present takes a pack position, so one approach to accommodating the new functionality would be to add a secondary bit position parameter, making the function's declaration look something like:

    static void write_reused_pack_one(struct packed_git *reuse_packfile,
                                      size_t pack_pos, size_t bitmap_pos,
                                      struct hashfile *out,
                                      off_t pack_start,
                                      struct pack_window **w_curs);

But because the pack-relative position can be easily derived from the bit position, it makes senes to just pass the latter and let the function itself translate it into a pack-relative position.

To do this, extract a new function `bitmap_to_pack_pos()` from the existing `write_reused_pack()` function. This new routine is responsible for performing the conversion from bitmap- to pack-relative positions.

Instead of performing that translation in `write_reused_pack()`, instead call the new function from within `write_reused_pack_one()` so that we can just pass a single bit position to it.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/pack-objects.c | 78 ++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 37 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 097bb5ac2ca..7f50d58a235 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1017,6 +1017,42 @@ static off_t find_reused_offset(off_t where)
 	return reused_chunks[lo-1].difference;
 }
 
+static uint32_t bitmap_to_pack_pos(struct packed_git *reuse_packfile,
+				   size_t pos)
+{
+	if (bitmap_is_midx(bitmap_git)) {
+		/*
+		 * When doing multi-pack reuse on a
+		 * non-preferred pack, translate bit positions
+		 * from the MIDX pseudo-pack order back to their
+		 * pack-relative positions before attempting
+		 * reuse.
+		 */
+		struct multi_pack_index *m = bitmap_midx(bitmap_git);
+		uint32_t midx_pos, pack_pos;
+		off_t pack_ofs;
+
+		if (!m)
+			BUG("non-zero bitmap position without MIDX");
+
+		midx_pos = pack_pos_to_midx(m, pos);
+		pack_ofs = nth_midxed_offset(m, midx_pos);
+
+		if (offset_to_pack_pos(reuse_packfile, pack_ofs, &pack_pos) < 0)
+			BUG("could not find expected object at offset %"PRIuMAX" in pack %s",
+			    (uintmax_t)pack_ofs, pack_basename(reuse_packfile));
+
+		return pack_pos;
+	} else {
+		/*
+		 * Can use bit positions directly, even for MIDX
+		 * bitmaps. See comment in try_partial_reuse()
+		 * for why.
+		 */
+		return pos;
+	}
+}
+
 static void write_reused_pack_one(struct packed_git *reuse_packfile,
 				  size_t pos, struct hashfile *out,
 				  off_t pack_start,
@@ -1025,9 +1061,10 @@ static void write_reused_pack_one(struct packed_git *reuse_packfile,
 	off_t offset, next, cur;
 	enum object_type type;
 	unsigned long size;
+	uint32_t pack_pos = bitmap_to_pack_pos(reuse_packfile, pos);
 
-	offset = pack_pos_to_offset(reuse_packfile, pos);
-	next = pack_pos_to_offset(reuse_packfile, pos + 1);
+	offset = pack_pos_to_offset(reuse_packfile, pack_pos);
+	next = pack_pos_to_offset(reuse_packfile, pack_pos + 1);
 
 	record_reused_object(offset,
 			     offset - (hashfile_total(out) - pack_start));
@@ -1191,7 +1228,6 @@ static void write_reused_pack(struct bitmapped_pack *reuse_packfile,
 		size_t pos = (i * BITS_IN_EWORD);
 
 		for (offset = 0; offset < BITS_IN_EWORD; ++offset) {
-			uint32_t pack_pos;
 			if ((word >> offset) == 0)
 				break;
 
@@ -1201,40 +1237,8 @@ static void write_reused_pack(struct bitmapped_pack *reuse_packfile,
 			if (pos + offset >= reuse_packfile->bitmap_pos + reuse_packfile->bitmap_nr)
 				goto done;
 
-			if (bitmap_is_midx(bitmap_git)) {
-				/*
-				 * When doing multi-pack reuse on a
-				 * non-preferred pack, translate bit positions
-				 * from the MIDX pseudo-pack order back to their
-				 * pack-relative positions before attempting
-				 * reuse.
-				 */
-				struct multi_pack_index *m = bitmap_midx(bitmap_git);
-				uint32_t midx_pos;
-				off_t pack_ofs;
-
-				if (!m)
-					BUG("non-zero bitmap position without MIDX");
-
-				midx_pos = pack_pos_to_midx(m, pos + offset);
-				pack_ofs = nth_midxed_offset(m, midx_pos);
-
-				if (offset_to_pack_pos(reuse_packfile->p,
-						       pack_ofs, &pack_pos) < 0)
-					BUG("could not find expected object at offset %"PRIuMAX" in pack %s",
-					    (uintmax_t)pack_ofs,
-					    pack_basename(reuse_packfile->p));
-			} else {
-				/*
-				 * Can use bit positions directly, even for MIDX
-				 * bitmaps. See comment in try_partial_reuse()
-				 * for why.
-				 */
-				pack_pos = pos + offset;
-			}
-
-			write_reused_pack_one(reuse_packfile->p, pack_pos, f,
-					      pack_start, &w_curs);
+			write_reused_pack_one(reuse_packfile->p, pos + offset,
+					      f, pack_start, &w_curs);
 			display_progress(progress_state, ++written);
 		}
 	}
-- 
2.47.0.11.g487258bca34
Previous: Taylor BlauNext: Jeff King
Message 8 of 31 in “pack-bitmap: convert offset to ref deltas where possible”
  1. 00/11 pack-bitmap: convert offset to ref deltas where possibleTaylor Blau, Oct 9, 2024
  2. 01/11 pack-bitmap.c: do not pass `pack_pos` to `try_partial_reuse()`Taylor Blau, Oct 9, 2024
  3. 02/11 pack-bitmap.c: avoid unnecessary `offset_to_pack_pos()`Taylor Blau, Oct 9, 2024
  4. 03/11 pack-bitmap.c: delay calling 'offset_to_pack_pos()'Taylor Blau, Oct 9, 2024
  5. 04/11 pack-bitmap.c: compare `base_offset` to `delta_obj_offset`Taylor Blau, Oct 9, 2024
  6. 05/11 pack-bitmap.c: extract `find_base_bitmap_pos()`Taylor Blau, Oct 9, 2024
  7. 06/11 pack-bitmap: drop `from_midx` field from `bitmapped_pack`Taylor Blau, Oct 9, 2024
  8. 07/11 write_reused_pack_one(): translate bit positions directlyTaylor Blau, Oct 9, 2024
  9. Jeff KingOct 11, 2024
  10. Taylor BlauNov 4, 2024
  11. 08/11 t5332: enable OFS_DELTAs via test_pack_objects_reusedTaylor Blau, Oct 9, 2024
  12. Jeff KingOct 11, 2024
  13. Taylor BlauNov 4, 2024
  14. 09/11 pack-bitmap: enable cross-pack delta reuseTaylor Blau, Oct 9, 2024
  15. Jeff KingOct 11, 2024
  16. Taylor BlauNov 4, 2024
  17. 10/11 pack-bitmap.c: record whether the result was filteredTaylor Blau, Oct 9, 2024
  18. Jeff KingOct 11, 2024
  19. Taylor BlauNov 4, 2024
  20. 11/11 pack-bitmap: enable reusing deltas with base objects in 'haves' bitmapTaylor Blau, Oct 9, 2024
  21. Junio C HamanoOct 10, 2024
  22. Taylor BlauOct 10, 2024
  23. Junio C HamanoOct 10, 2024
  24. Taylor BlauOct 10, 2024
  25. Jeff KingOct 11, 2024
  26. Jeff KingOct 11, 2024
  27. Junio C HamanoOct 11, 2024
  28. Jeff KingOct 11, 2024
  29. Taylor BlauNov 19, 2024
  30. Taylor BlauNov 19, 2024
  31. Jeff KingDec 18, 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.