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

[PATCH 02/11] pack-bitmap.c: avoid unnecessary `offset_to_pack_pos()`

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

In `try_partial_reuse()`, for any object which is either an OFS_DELTA or REF_DELTA, we need to determine the bitmap position for its base.

We handle the case of single- and multi-pack bitmaps separately:
  - For multi-pack bitmaps, we look to see if the MIDX selected the
    base we're looking at to represent that object. If the base object
    was selected from a different pack in the MIDX, we won't reuse the
    delta.
  - For single-pack bitmaps, we convert the base object's offset into
    a pack-relative position (identified here as 'base_pos') by
    calling `offset_to_pack_pos()`.

But we also call `offset_to_pack_pos()` before handling each case above separately. For the MIDX case, this is unnecessary, since we don't need to lookup the base object's pack-relative position; instead, we just care whether the selected copy of that base is from the same pack we're currently operating on.

For the non-MIDX case, we end up calling offset_to_pack_pos() again, yielding the same result, making the earlier call wasteful. This behavior dates back to 519e17ff75 (pack-bitmap: prepare to mark objects from multiple packs for reuse, 2023-12-14).

Let's correct that by avoiding the unconditional call to 'offset_to_pack_pos()'.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 pack-bitmap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 706ff26a7b1..5c5c26efe0d 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2069,7 +2069,6 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
 
 	if (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA) {
 		off_t base_offset;
-		uint32_t base_pos;
 		uint32_t base_bitmap_pos;
 
 		/*
@@ -2085,8 +2084,6 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
 		if (!base_offset)
 			return 0;
 
-		offset_to_pack_pos(pack->p, base_offset, &base_pos);
-
 		if (bitmap_is_midx(bitmap_git)) {
 			/*
 			 * Cross-pack deltas are rejected for now, but could
@@ -2105,6 +2102,8 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
 				return 0;
 			}
 		} else {
+			uint32_t base_pos;
+
 			if (offset_to_pack_pos(pack->p, base_offset,
 					       &base_pos) < 0)
 				return 0;
-- 
2.47.0.11.g487258bca34
Previous: Taylor BlauNext: Taylor Blau
Message 3 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.