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

[PATCH v2 1/1] repack: avoid loosening promisor objects in partial clones

From
Rafael Silva <rafaeloliveira.cs@gmail.com>
Date
Apr 18, 2021, 13:57 UTC
Message-ID
<20210418135749.27152-2-rafaeloliveira.cs@gmail.com>
In-Reply-To
<20210418135749.27152-1-rafaeloliveira.cs@gmail.com>

When `git repack -A -d` is run in a partial clone, `pack-objects` is invoked twice: once to repack all promisor objects, and once to repack all non-promisor objects. The latter `pack-objects` invocation is with --exclude-promisor-objects and --unpack-unreachable, which loosens all unused objects. Unfortunately, this includes promisor objects.

Because the -d argument to `git repack` subsequently deletes all loose objects also in packs, these just-loosened promisor objects will be immediately deleted. However, this extra disk churn is unnecessary in the first place. For example, a newly-clone partial repo that filters all blob objects (e.g. `--filter=blob:none`), `repack` ends up unpacking all trees and commits into the filesystem because every object, in this particular case, is a promisor object. Depending on the repo size, this increases the disk usage considerably: In my copy of the linux.git, the object directory peaked 26GB of more disk usage.

In order to avoid this extra disk churn, pass the names of the promisor packfiles as --keep-pack arguments to the second invocation of `pack-objects`. This informs `pack-objects` that the promisor objects are already in a safe packfile and, therefore, do not need to be loosened. The --keep-pack option takes only a packfile name, but we concatenate both the path and the name in a single string. Instead, let's split them into separate string in order to easily pass the packfile name later.

For testing, we need to validate whether any object was loosened. However, the "evidence" (loosened objects) is deleted during the process which prevents us from inspecting the object directory. Instead, let's teach `pack-objects` to count loosened objects and emit via trace2 thus allowing inspecting the debug events after the process is finished. This new event is used on the added regression test.

Lastly, add a new perf test to evaluate the performance impact made by this changes (tested on git.git):

     Test          HEAD^                 HEAD
     ----------------------------------------------------------
     5600.3: gc    134.38(41.93+90.95)   7.80(6.72+1.35) -94.2%

For a bigger repository, such as linux.git, the improvement is even bigger:

     Test          HEAD^                     HEAD
     -------------------------------------------------------------------
     5600.3: gc    6833.00(918.07+3162.74)   268.79(227.02+39.18) -96.1%

These improvements are particular big because every object in the newly-cloned partial repository is a promisor object.

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
---
 builtin/pack-objects.c        | 8 +++++++-
 builtin/repack.c              | 9 +++++++--
 t/perf/p5600-partial-clone.sh | 4 ++++
 t/t5616-partial-clone.sh      | 8 ++++++++
 4 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 40ee6fa19f..73889cec95 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3479,6 +3479,7 @@ static void loosen_unused_packed_objects(void)
 {
 	struct packed_git *p;
 	uint32_t i;
+	uint32_t loosened_objects_nr = 0;
 	struct object_id oid;
 
 	for (p = get_all_packs(the_repository); p; p = p->next) {
@@ -3492,11 +3493,16 @@ static void loosen_unused_packed_objects(void)
 			nth_packed_object_id(&oid, p, i);
 			if (!packlist_find(&to_pack, &oid) &&
 			    !has_sha1_pack_kept_or_nonlocal(&oid) &&
-			    !loosened_object_can_be_discarded(&oid, p->mtime))
+			    !loosened_object_can_be_discarded(&oid, p->mtime)) {
 				if (force_object_loose(&oid, p->mtime))
 					die(_("unable to force loose object"));
+				loosened_objects_nr++;
+			}
 		}
 	}
+
+	trace2_data_intmax("pack-objects", the_repository,
+			   "loosen_unused_packed_objects/loosened", loosened_objects_nr);
 }
 
 /*
diff --git a/builtin/repack.c b/builtin/repack.c
index 2847fdfbab..5f9bc74adc 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -20,7 +20,7 @@ static int delta_base_offset = 1;
 static int pack_kept_objects = -1;
 static int write_bitmaps = -1;
 static int use_delta_islands;
-static char *packdir, *packtmp;
+static char *packdir, *packtmp_name, *packtmp;
 
 static const char *const git_repack_usage[] = {
 	N_("git repack [<options>]"),
@@ -530,7 +530,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	}
 
 	packdir = mkpathdup("%s/pack", get_object_directory());
-	packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid());
+	packtmp_name = xstrfmt(".tmp-%d-pack", (int)getpid());
+	packtmp = mkpathdup("%s/%s", packdir, packtmp_name);
 
 	sigchain_push_common(remove_pack_on_signal);
 
@@ -573,6 +574,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 		repack_promisor_objects(&po_args, &names);
 
 		if (existing_packs.nr && delete_redundant) {
+			for_each_string_list_item(item, &names) {
+				strvec_pushf(&cmd.args, "--keep-pack=%s-%s.pack",
+					     packtmp_name, item->string);
+			}
 			if (unpack_unreachable) {
 				strvec_pushf(&cmd.args,
 					     "--unpack-unreachable=%s",
diff --git a/t/perf/p5600-partial-clone.sh b/t/perf/p5600-partial-clone.sh
index ca785a3341..a965f2c4d6 100755
--- a/t/perf/p5600-partial-clone.sh
+++ b/t/perf/p5600-partial-clone.sh
@@ -35,4 +35,8 @@ test_perf 'count non-promisor commits' '
 	git -C bare.git rev-list --all --count --exclude-promisor-objects
 '
 
+test_perf 'gc' '
+	git -C bare.git gc
+'
+
 test_done
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 5cb415386e..6e3e7565d0 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -548,6 +548,14 @@ test_expect_success 'fetch from a partial clone, protocol v2' '
 	grep "version 2" trace
 '
 
+test_expect_success 'repack does not loosen promisor objects' '
+	rm -rf client trace &&
+	git clone --bare --filter=blob:none "file://$(pwd)/srv.bare" client &&
+	test_when_finished "rm -rf client trace" &&
+	GIT_TRACE2_PERF="$(pwd)/trace" git -C client repack -A -d &&
+	grep "loosen_unused_packed_objects/loosened:0" trace
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 

-- 
2.31.0.699.g8849f49b87
Previous: Rafael SilvaNext: Jonathan Tan
Message 41 of 46 in “rather slow 'git repack' in 'blob:none' partial clones”
  1. SZEDER GáborApr 3, 2021
  2. Rafael SilvaApr 5, 2021
  3. Jeff KingApr 7, 2021
  4. Jonathan TanApr 8, 2021
  5. Jeff KingApr 8, 2021
  6. Rafael SilvaApr 12, 2021
  7. SZEDER GáborApr 12, 2021
  8. Bryan TurnerApr 12, 2021
  9. Jeff KingApr 12, 2021
  10. Jeff KingApr 12, 2021
  11. 0/3 low-hanging performance fruit with promisor packsJeff King, Apr 13, 2021
  12. 1/3 is_promisor_object(): free tree buffer after parsingJeff King, Apr 13, 2021
  13. Junio C HamanoApr 13, 2021
  14. Jeff KingApr 14, 2021
  15. 2/3 lookup_unknown_object(): take a repository argumentJeff King, Apr 13, 2021
  16. 3/3 revision: avoid parsing with --exclude-promisor-objectsJeff King, Apr 13, 2021
  17. Junio C HamanoApr 13, 2021
  18. SZEDER GáborApr 13, 2021
  19. Jonathan TanApr 14, 2021
  20. Rafael SilvaApr 14, 2021
  21. SZEDER GáborApr 13, 2021
  22. Jeff KingApr 14, 2021
  23. SZEDER GáborApr 11, 2021
  24. Rafael SilvaApr 12, 2021
  25. 0/2 prevent `repack` to unpack and delete promisor objectsRafael Silva, Apr 14, 2021
  26. 1/2 repack: teach --no-prune-packed to skip `git prune-packed`Rafael Silva, Apr 14, 2021
  27. Jonathan TanApr 14, 2021
  28. Rafael SilvaApr 18, 2021
  29. 2/2 repack: avoid loosening promisor pack objects in partial clonesRafael Silva, Apr 14, 2021
  30. Jonathan TanApr 15, 2021
  31. Junio C HamanoApr 15, 2021
  32. Jeff KingApr 15, 2021
  33. Jeff KingApr 15, 2021
  34. Rafael SilvaApr 18, 2021
  35. Junio C HamanoApr 15, 2021
  36. Rafael SilvaApr 18, 2021
  37. Junio C HamanoApr 14, 2021
  38. Jeff KingApr 15, 2021
  39. Rafael SilvaApr 18, 2021
  40. 0/1 prevent `repack` to unpack and delete promisor objectsRafael Silva, Apr 18, 2021
  41. 1/1 repack: avoid loosening promisor objects in partial clonesRafael Silva, Apr 18, 2021
  42. Jonathan TanApr 19, 2021
  43. Rafael SilvaApr 21, 2021
  44. Junio C HamanoApr 19, 2021
  45. Rafael SilvaApr 21, 2021
  46. repack: avoid loosening promisor objects in partial clonesRafael Silva, Apr 21, 2021

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.