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

[PATCH v2 6/7] pack-objects: compute local/ignore_pack_keep early

From
Jeff King <peff@peff.net>
Date
Jul 29, 2016, 04:11 UTC
Message-ID
<20160729041130.GF22408@sigill.intra.peff.net>
In-Reply-To
<20160729040422.GA19678@sigill.intra.peff.net>

In want_object_in_pack(), we can exit early from our loop if neither "local" nor "ignore_pack_keep" are set. If they are, however, we must examine each pack to see if it has the object and is non-local or has a ".keep".

It's quite common for there to be no non-local or .keep packs at all, in which case we know ahead of time that looking further will be pointless. We can pre-compute this by simply iterating over the list of packs ahead of time, and dropping the flags if there are no packs that could match.

Another similar strategy would be to modify the loop in want_object_in_pack() to notice that we have already found the object once, and that we are looping only to check for "local" and "keep" attributes. If a pack has neither of those, we can skip the call to find_pack_entry_one(), which is the expensive part of the loop.

This has two advantages:
  - it isn't all-or-nothing; we still get some improvement
    when there's a small number of kept or non-local packs,
    and a large number of non-kept local packs
  - it eliminates any possible race where we add new
    non-local or kept packs after our initial scan. In
    practice, I don't think this race matters; we already
    cache the packed_git information, so somebody who adds a
    new pack or .keep file after we've started will not be
    noticed at all, unless we happen to need to call
    reprepare_packed_git() because a lookup fails.
    In other words, we're already racy, and the race is not
    a big deal (losing the race means we might include an
    object in the pack that would not otherwise be, which is
    an acceptable outcome).

However, it also has a disadvantage: we still loop over the rest of the packs for each object to check their flags. This is much less expensive than doing the object lookup, but still not free. So if we wanted to implement that strategy to cover the non-all-or-nothing cases, we could do so in addition to this one (so you get the most speedup in the all-or-nothing case, and the best we can do in the other cases). But given that the all-or-nothing case is likely the most common, it is probably not worth the trouble, and we can revisit this later if evidence points otherwise.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/pack-objects.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8ad11f2..c4c2a3c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -46,6 +46,7 @@ static int keep_unreachable, unpack_unreachable, include_tag;
 static unsigned long unpack_unreachable_expiration;
 static int pack_loose_unreachable;
 static int local;
+static int have_non_local_packs;
 static int incremental;
 static int ignore_packed_keep;
 static int allow_ofs_delta;
@@ -990,7 +991,8 @@ static int want_object_in_pack(const unsigned char *sha1,
 			 * we just found is going to be packed, so break
 			 * out of the loop to return 1 now.
 			 */
-			if (!ignore_packed_keep && !local)
+			if (!ignore_packed_keep &&
+			    (!local || !have_non_local_packs))
 				break;
 
 			if (local && !p->pack_local)
@@ -2799,6 +2801,28 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 		progress = 2;
 
 	prepare_packed_git();
+	if (ignore_packed_keep) {
+		struct packed_git *p;
+		for (p = packed_git; p; p = p->next)
+			if (p->pack_local && p->pack_keep)
+				break;
+		if (!p) /* no keep-able packs found */
+			ignore_packed_keep = 0;
+	}
+	if (local) {
+		/*
+		 * unlike ignore_packed_keep above, we do not want to
+		 * unset "local" based on looking at packs, as it
+		 * also covers non-local objects
+		 */
+		struct packed_git *p;
+		for (p = packed_git; p; p = p->next) {
+			if (!p->pack_local) {
+				have_non_local_packs = 1;
+				break;
+			}
+		}
+	}
 
 	if (progress)
 		progress_state = start_progress(_("Counting objects"), 0);
-- 
2.9.2.607.g98dce7b
Previous: Jeff KingNext: Jeff King
Message 7 of 35 in “speed up pack-objects counting with many packs”
  1. 0/7 speed up pack-objects counting with many packsJeff King, Jul 29, 2016
  2. 1/7 t/perf: add tests for many-pack scenariosJeff King, Jul 29, 2016
  3. 2/7 sha1_file: drop free_pack_by_nameJeff King, Jul 29, 2016
  4. 3/7 add generic most-recently-used listJeff King, Jul 29, 2016
  5. 4/7 find_pack_entry: replace last_found_pack with MRU cacheJeff King, Jul 29, 2016
  6. 5/7 pack-objects: break out of want_object loop earlyJeff King, Jul 29, 2016
  7. 6/7 pack-objects: compute local/ignore_pack_keep earlyJeff King, Jul 29, 2016
  8. 7/7 pack-objects: use mru list when iterating over packsJeff King, Jul 29, 2016
  9. Jeff KingJul 29, 2016
  10. Junio C HamanoJul 29, 2016
  11. Jeff KingAug 8, 2016
  12. Junio C HamanoAug 8, 2016
  13. Jeff KingAug 8, 2016
  14. Junio C HamanoAug 8, 2016
  15. Jeff KingAug 9, 2016
  16. Jeff KingAug 9, 2016
  17. Junio C HamanoAug 9, 2016
  18. Junio C HamanoAug 9, 2016
  19. 0/2 pack-objects mruJeff King, Aug 10, 2016
  20. Junio C HamanoAug 10, 2016
  21. Jeff KingAug 11, 2016
  22. 1/2 pack-objects: break delta cycles before delta-search phaseJeff King, Aug 10, 2016
  23. Junio C HamanoAug 10, 2016
  24. Jeff KingAug 11, 2016
  25. 1/2 pack-objects: break delta cycles before delta-search phaseJeff King, Aug 11, 2016
  26. Jeff KingAug 11, 2016
  27. pack-objects mruJeff King, Aug 11, 2016
  28. 1/4 provide an initializer for "struct object_info"Jeff King, Aug 11, 2016
  29. 2/4 sha1_file: make packed_object_info publicJeff King, Aug 11, 2016
  30. 3/4 pack-objects: break delta cycles before delta-search phaseJeff King, Aug 11, 2016
  31. 4/4 pack-objects: use mru list when iterating over packsJeff King, Aug 11, 2016
  32. Jeff KingAug 11, 2016
  33. Junio C HamanoAug 11, 2016
  34. Jeff KingAug 11, 2016
  35. 2/2 pack-objects: use mru list when iterating over packsJeff King, Aug 10, 2016

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.