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

[PATCH v2 1/1] builtin/pack-objects.c: avoid iterating all refs

From
Jacob Vosmaer <jacob@gitlab.com>
Date
Jan 20, 2021, 12:45 UTC
Message-ID
<20210120124514.49737-2-jacob@gitlab.com>
In-Reply-To
<20210120124514.49737-1-jacob@gitlab.com>

In git-pack-objects, we iterate over all the tags if the --include-tag option is passed on the command line. For some reason this uses for_each_ref which is expensive if the repo has many refs. We should use for_each_tag_ref instead.

Because the add_ref_tag callback will now only visit tags we simplified it a bit.

The motivation for this change is that we observed performance issues with a repository on gitlab.com that has 500,000 refs but only 2,000 tags. The fetch traffic on that repo is dominated by CI, and when we changed CI to fetch with 'git fetch --no-tags' we saw a dramatic change in the CPU profile of git-pack-objects. This lead us to this particular ref walk. More details in: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/746#note_483546598

Signed-off-by: Jacob Vosmaer <jacob@gitlab.com>
---
 builtin/pack-objects.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 2a00358f34..ad52c91bdb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2803,13 +2803,11 @@ static void add_tag_chain(const struct object_id *oid)
 	}
 }
 
-static int add_ref_tag(const char *path, const struct object_id *oid, int flag, void *cb_data)
+static int add_ref_tag(const char *tag, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct object_id peeled;
 
-	if (starts_with(path, "refs/tags/") && /* is a tag? */
-	    !peel_ref(path, &peeled)    && /* peelable? */
-	    obj_is_packed(&peeled)) /* object packed? */
+	if (!peel_ref(tag, &peeled) && obj_is_packed(&peeled))
 		add_tag_chain(oid);
 	return 0;
 }
@@ -3740,7 +3738,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	}
 	cleanup_preferred_base();
 	if (include_tag && nr_result)
-		for_each_ref(add_ref_tag, NULL);
+		for_each_tag_ref(add_ref_tag, NULL);
 	stop_progress(&progress_state);
 	trace2_region_leave("pack-objects", "enumerate-objects",
 			    the_repository);
-- 
2.30.0
Previous: Jacob VosmaerNext: Taylor Blau
Message 2 of 11 in “builtin/pack-objects.c: avoid iterating all refs”
  1. 0/1 builtin/pack-objects.c: avoid iterating all refsJacob Vosmaer, Jan 20, 2021
  2. 1/1 builtin/pack-objects.c: avoid iterating all refsJacob Vosmaer, Jan 20, 2021
  3. Taylor BlauJan 20, 2021
  4. Jeff KingJan 20, 2021
  5. Taylor BlauJan 20, 2021
  6. Jacob VosmaerJan 20, 2021
  7. Jeff KingJan 20, 2021
  8. Jacob VosmaerJan 20, 2021
  9. Taylor BlauJan 20, 2021
  10. Jeff KingJan 21, 2021
  11. Jacob VosmaerJan 22, 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.