[PATCH 05/11] pack-bitmap.c: extract `find_base_bitmap_pos()`

From

Taylor Blau <me@ttaylorr.com>

Date

Oct 9, 2024, 20:31 UTC

Message-ID

<f4dca03d84583b5e8565af3b6759f05cf2b0be3f.1728505840.git.me@ttaylorr.com>

In-Reply-To

<cover.1728505840.git.me@ttaylorr.com>

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 pack-bitmap.c | 90 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 49 insertions(+), 41 deletions(-)

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 3e1034cabf3..6dbe6a2c5bc 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2047,6 +2047,52 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 	return NULL;
 }
 
+static int find_base_bitmap_pos(struct bitmap_index *bitmap_git,
+				struct bitmapped_pack *pack,
+				off_t base_offset,
+				off_t delta_obj_offset,
+				uint32_t *base_bitmap_pos)
+{
+	if (bitmap_is_midx(bitmap_git)) {
+		/*
+		 * Cross-pack deltas are rejected for now, but could
+		 * theoretically be supported in the future.
+		 *
+		 * We would need to ensure that we're sending both
+		 * halves of the delta/base pair, regardless of whether
+		 * or not the two cross a pack boundary. If they do,
+		 * then we must convert the delta to an REF_DELTA to
+		 * refer back to the base in the other pack.
+		 * */
+		if (midx_pair_to_pack_pos(bitmap_git->midx, pack->pack_int_id,
+					  base_offset, base_bitmap_pos) < 0)
+			return -1;
+	} else {
+		/*
+		 * We assume delta dependencies always point backwards.
+		 * This lets us do a single pass, and is basically
+		 * always true due to the way OFS_DELTAs work. You would
+		 * not typically find REF_DELTA in a bitmapped pack,
+		 * since we only bitmap packs we write fresh, and
+		 * OFS_DELTA is the default). But let's double check to
+		 * make sure the pack wasn't written with odd
+		 * parameters.
+		 *
+		 * Since we're working on a single-pack bitmap, we can
+		 * use the object offset as a proxy for the bit
+		 * position, since the bits are ordered by their
+		 * positions within the pack.
+		 */
+		if (base_offset >= delta_obj_offset)
+			return -1;
+		if (offset_to_pack_pos(pack->p, base_offset,
+				       base_bitmap_pos) < 0)
+			return -1;
+	}
+
+	return 0;
+}
+
 /*
  * -1 means "stop trying further objects"; 0 means we may or may not have
  * reused, but you can keep feeding bits.
@@ -2081,49 +2127,11 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
 		 */
 		base_offset = get_delta_base(pack->p, w_curs, &offset, type,
 					     delta_obj_offset);
-		if (!base_offset)
+		if (!base_offset ||
+		    find_base_bitmap_pos(bitmap_git, pack, base_offset,
+					 delta_obj_offset, &base_bitmap_pos) < 0)
 			return 0;
 
-		if (bitmap_is_midx(bitmap_git)) {
-			/*
-			 * Cross-pack deltas are rejected for now, but could
-			 * theoretically be supported in the future.
-			 *
-			 * We would need to ensure that we're sending both
-			 * halves of the delta/base pair, regardless of whether
-			 * or not the two cross a pack boundary. If they do,
-			 * then we must convert the delta to an REF_DELTA to
-			 * refer back to the base in the other pack.
-			 * */
-			if (midx_pair_to_pack_pos(bitmap_git->midx,
-						  pack->pack_int_id,
-						  base_offset,
-						  &base_bitmap_pos) < 0) {
-				return 0;
-			}
-		} else {
-			/*
-			 * We assume delta dependencies always point backwards.
-			 * This lets us do a single pass, and is basically
-			 * always true due to the way OFS_DELTAs work. You would
-			 * not typically find REF_DELTA in a bitmapped pack,
-			 * since we only bitmap packs we write fresh, and
-			 * OFS_DELTA is the default). But let's double check to
-			 * make sure the pack wasn't written with odd
-			 * parameters.
-			 *
-			 * Since we're working on a single-pack bitmap, we can
-			 * use the object offset as a proxy for the bit
-			 * position, since the bits are ordered by their
-			 * positions within the pack.
-			 */
-			if (base_offset >= delta_obj_offset)
-				return 0;
-			if (offset_to_pack_pos(pack->p, base_offset,
-					       &base_bitmap_pos) < 0)
-				return 0;
-		}
-
 		/*
 		 * And finally, if we're not sending the base as part of our
 		 * reuse chunk, then don't send this object either. The base

-- 
2.47.0.11.g487258bca34