[PATCH 0/2] fetch-pack: allow parallelizing packfile URI fetches

[PATCH 0/2] fetch-pack: allow parallelizing packfile URI fetches

[Patrick Steinhardt]

Hi,

this patch series prepares git-fetch(1) and git-clone(1) to handle
fetches of packfile URIs in parallel. This can significantly speed up
fetches when the server announces a bunch of packfiles, as shown in the
benchmarks in the second patch.

Thanks!

Patrick

---
Patrick Steinhardt (2):
      fetch-pack: prepare for threaded fetching of packfile URIs
      fetch-pack: allow parallelizing packfile URI fetches

 Documentation/config/fetch.adoc |   9 ++
 fetch-pack.c                    | 228 ++++++++++++++++++++++++++++++----------
 t/t5702-protocol-v2.sh          |  44 ++++++++
 3 files changed, 227 insertions(+), 54 deletions(-)


---
base-commit: 1a3e64c6c4a623626ff0687008732a8e007e2a1c
change-id: 20260821-pks-parallelize-fetching-packfile-uris-b1ad24a82fe0

[PATCH 1/2] fetch-pack: prepare for threaded fetching of packfile URIs

[Patrick Steinhardt]

In the next commit, we're about to add the ability to parallelize
fetching packfile URIs. Refactor the code to prepare for this by
splitting the logic up into three explicit phases:

  1. Preparation phase, where we allocate the state that will be
     populated by the different threads.

  2. Fetch phase, where we fetch the packfile URIs. This is the part
     that will be parallelized, and we need to be careful to not access
     any shared state here.

  3. Aggregation phase, where we aggregate results from the parallel
     worker threads.

This should not result in a user-visible change in behaviour.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 fetch-pack.c | 148 +++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 94 insertions(+), 54 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 626f799712..6aca0b2588 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1668,6 +1668,98 @@ static void do_check_stateless_delimiter(int stateless_rpc,
 				  _("git fetch-pack: expected response end packet"));
 }
 
+struct fetch_packfile_uri_result {
+	struct oidset gitmodules_found;
+	char packhash[GIT_MAX_HEXSZ + 1];
+	bool created_keep;
+};
+
+static void fetch_packfile_uri(const char *uri_with_hash,
+			       const struct strvec *index_pack_args,
+			       struct fetch_packfile_uri_result *result)
+{
+	struct child_process cmd = CHILD_PROCESS_INIT;
+	const char *uri = uri_with_hash +
+		the_hash_algo->hexsz + 1;
+
+	strvec_push(&cmd.args, "http-fetch");
+	strvec_pushf(&cmd.args, "--packfile=%.*s",
+		     (int) the_hash_algo->hexsz, uri_with_hash);
+	for (size_t j = 0; j < index_pack_args->nr; j++)
+		strvec_pushf(&cmd.args, "--index-pack-arg=%s",
+			     index_pack_args->v[j]);
+	strvec_push(&cmd.args, uri);
+	cmd.git_cmd = 1;
+	cmd.no_stdin = 1;
+	cmd.out = -1;
+	if (start_command(&cmd))
+		die("fetch-pack: unable to spawn http-fetch");
+
+	if (read_in_full(cmd.out, result->packhash, 5) != 5 ||
+	    (memcmp(result->packhash, "keep\t", 5) &&
+	     memcmp(result->packhash, "pack\t", 5)))
+		die("fetch-pack: expected pack or keep then TAB at start of http-fetch output");
+	result->created_keep = !memcmp(result->packhash, "keep\t", 5);
+
+	if (read_in_full(cmd.out, result->packhash,
+			 the_hash_algo->hexsz + 1) != the_hash_algo->hexsz + 1 ||
+	    result->packhash[the_hash_algo->hexsz] != '\n')
+		die("fetch-pack: expected hash then LF in http-fetch output");
+	result->packhash[the_hash_algo->hexsz] = '\0';
+
+	parse_gitmodules_oids(cmd.out, &result->gitmodules_found);
+
+	close(cmd.out);
+
+	if (finish_command(&cmd))
+		die("fetch-pack: unable to finish http-fetch");
+
+	if (memcmp(uri_with_hash, result->packhash, the_hash_algo->hexsz))
+		die("fetch-pack: pack downloaded from %s does not match expected hash %.*s",
+		    uri, (int) the_hash_algo->hexsz,
+		    uri_with_hash);
+}
+
+static void fetch_packfile_uris(const struct string_list *packfile_uris,
+				const struct strvec *index_pack_args,
+				struct oidset *gitmodules_found,
+				struct string_list *pack_lockfiles)
+{
+	struct fetch_packfile_uri_result *results;
+
+	/* Initialize the data. */
+	CALLOC_ARRAY(results, packfile_uris->nr);
+	for (size_t i = 0; i < packfile_uris->nr; i++)
+		oidset_init(&results[i].gitmodules_found, 0);
+
+	/* Perform the fetches. */
+	for (size_t i = 0; i < packfile_uris->nr; i++)
+		fetch_packfile_uri(packfile_uris->items[i].string,
+				   index_pack_args, &results[i]);
+
+	/* Aggregate results. */
+	for (size_t i = 0; i < packfile_uris->nr; i++) {
+		struct fetch_packfile_uri_result *result = &results[i];
+		const struct object_id *oid;
+		struct oidset_iter iter;
+
+		if (result->created_keep) {
+			char *lockfile = xstrfmt("%s/pack/pack-%s.keep",
+						 repo_get_object_directory(the_repository),
+						 result->packhash);
+			string_list_append_nodup(pack_lockfiles, lockfile);
+		}
+
+		oidset_iter_init(&result->gitmodules_found, &iter);
+		while ((oid = oidset_iter_next(&iter)))
+			oidset_insert(gitmodules_found, oid);
+
+		oidset_clear(&result->gitmodules_found);
+	}
+
+	free(results);
+}
+
 static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 				    int fd[2],
 				    const struct ref *orig_ref,
@@ -1692,7 +1784,6 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 	struct object_id common_oid;
 	int received_ready = 0;
 	struct string_list packfile_uris = STRING_LIST_INIT_DUP;
-	int i;
 	struct strvec index_pack_args = STRVEC_INIT;
 	const char *promisor_remote_config;
 
@@ -1853,59 +1944,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 		}
 	}
 
-	for (i = 0; i < packfile_uris.nr; i++) {
-		bool created_keep;
-		int j;
-		struct child_process cmd = CHILD_PROCESS_INIT;
-		char packhash[GIT_MAX_HEXSZ + 1];
-		const char *uri = packfile_uris.items[i].string +
-			the_hash_algo->hexsz + 1;
-
-		strvec_push(&cmd.args, "http-fetch");
-		strvec_pushf(&cmd.args, "--packfile=%.*s",
-			     (int) the_hash_algo->hexsz,
-			     packfile_uris.items[i].string);
-		for (j = 0; j < index_pack_args.nr; j++)
-			strvec_pushf(&cmd.args, "--index-pack-arg=%s",
-				     index_pack_args.v[j]);
-		strvec_push(&cmd.args, uri);
-		cmd.git_cmd = 1;
-		cmd.no_stdin = 1;
-		cmd.out = -1;
-		if (start_command(&cmd))
-			die("fetch-pack: unable to spawn http-fetch");
-
-		if (read_in_full(cmd.out, packhash, 5) != 5 ||
-		    (memcmp(packhash, "keep\t", 5) &&
-		     memcmp(packhash, "pack\t", 5)))
-			die("fetch-pack: expected pack or keep then TAB at start of http-fetch output");
-		created_keep = !memcmp(packhash, "keep\t", 5);
-
-		if (read_in_full(cmd.out, packhash,
-				 the_hash_algo->hexsz + 1) != the_hash_algo->hexsz + 1 ||
-		    packhash[the_hash_algo->hexsz] != '\n')
-			die("fetch-pack: expected hash then LF in http-fetch output");
-		packhash[the_hash_algo->hexsz] = '\0';
-
-		parse_gitmodules_oids(cmd.out, &fsck_options.gitmodules_found);
-
-		close(cmd.out);
-
-		if (finish_command(&cmd))
-			die("fetch-pack: unable to finish http-fetch");
-
-		if (memcmp(packfile_uris.items[i].string, packhash,
-			   the_hash_algo->hexsz))
-			die("fetch-pack: pack downloaded from %s does not match expected hash %.*s",
-			    uri, (int) the_hash_algo->hexsz,
-			    packfile_uris.items[i].string);
-
-		if (created_keep)
-			string_list_append_nodup(pack_lockfiles,
-						 xstrfmt("%s/pack/pack-%s.keep",
-							 repo_get_object_directory(the_repository),
-							 packhash));
-	}
+	fetch_packfile_uris(&packfile_uris, &index_pack_args,
+			    &fsck_options.gitmodules_found, pack_lockfiles);
 	string_list_clear(&packfile_uris, 0);
 	strvec_clear(&index_pack_args);
 

-- 
2.55.0.822.g20453c30eb.dirty

[PATCH 2/2] fetch-pack: allow parallelizing packfile URI fetches

[Patrick Steinhardt]

When cloning from a server that supports packfile URIs we may see
multiple URIs being announced by the server. If so, the expectation is
that the client will download all of those packfiles. This is being done
sequentially, where we fetch one packfile after the other.

In many cases this should be fine, but there are scenarios where it's
not. When packfiles are for example hosted by object storage (think AWS
S3 or GCS) then the way to achieve high performance is typically to
parallelize downloading the data as a single connection is often capped
at a certain bandwidth. Furthermore, when the server announces a bunch
of smaller packfiles, then the overhead of establishing the connection
may eventually add up.

Despite the limitations caused by the network bandwidth and latency, Git
also runs git-index-pack(1) on all of the fetched packfiles. This is
another task that can be easily parallelized for another speedup.

All of these limitations can be addressed by parallelizing the fetch.
Introduce a new configuration option that allows the user to ask for
this: by default we continue to not parallelize the fetch to retain the
status quo. But when configured to 0 (where we auto-detect the number of
cores) or a value larger than 1 we perform the fetches concurrently.

With this infrastructure in place we can significantly speed up such
fetches. Using a local HTTP server demonstrates the speedup when using a
throttled connection of 2MB/s and downloading 8x1MB packfiles:

    Benchmark 1: 2MB/s, 8x1MB packfiles, 1 threads
      Time (mean ± σ):      4.321 s ±  0.003 s    [User: 0.195 s, System: 0.113 s]
      Range (min … max):    4.318 s …  4.325 s    5 runs

    Benchmark 2: 2MB/s, 8x1MB packfiles, 2 threads
      Time (mean ± σ):      2.284 s ±  0.241 s    [User: 0.191 s, System: 0.114 s]
      Range (min … max):    2.173 s …  2.714 s    5 runs

    Benchmark 3: 2MB/s, 8x1MB packfiles, 4 threads
      Time (mean ± σ):      1.212 s ±  0.238 s    [User: 0.192 s, System: 0.105 s]
      Range (min … max):    1.102 s …  1.638 s    5 runs

    Benchmark 4: 2MB/s, 8x1MB packfiles, 8 threads
      Time (mean ± σ):     569.9 ms ±   2.5 ms    [User: 183.4 ms, System: 116.0 ms]
      Range (min … max):   566.5 ms … 573.4 ms    5 runs

    Summary
      2MB/s, 8x1MB packfiles, 8 threads ran
        2.13 ± 0.42 times faster than 2MB/s, 8x1MB packfiles, 4 threads
        4.01 ± 0.42 times faster than 2MB/s, 8x1MB packfiles, 2 threads
        7.58 ± 0.03 times faster than 2MB/s, 8x1MB packfiles, 1 threads

Quite unsurprisingly, we scale almost linearly with the number of
threads in this case as we're limited by the bandwidth of a single
connection. But we can also demonstrate a speedup on an unthrottled
connection when downloading slightly larger packfiles:

    Benchmark 1: unthrottled, 8x16MB packfiles, 1 threads
      Time (mean ± σ):      2.434 s ±  0.031 s    [User: 2.067 s, System: 0.329 s]
      Range (min … max):    2.381 s …  2.460 s    5 runs

    Benchmark 2: unthrottled, 8x16MB packfiles, 2 threads
      Time (mean ± σ):      1.353 s ±  0.129 s    [User: 2.025 s, System: 0.328 s]
      Range (min … max):    1.288 s …  1.583 s    5 runs

    Benchmark 3: unthrottled, 8x16MB packfiles, 4 threads
      Time (mean ± σ):     702.9 ms ±  23.9 ms    [User: 1732.5 ms, System: 313.9 ms]
      Range (min … max):   660.7 ms … 718.5 ms    5 runs

    Benchmark 4: unthrottled, 8x16MB packfiles, 8 threads
      Time (mean ± σ):     455.1 ms ±   7.7 ms    [User: 1730.0 ms, System: 372.3 ms]
      Range (min … max):   442.8 ms … 462.8 ms    5 runs

    Summary
      unthrottled, 8x16MB packfiles, 8 threads ran
        1.54 ± 0.06 times faster than unthrottled, 8x16MB packfiles, 4 threads
        2.97 ± 0.29 times faster than unthrottled, 8x16MB packfiles, 2 threads
        5.35 ± 0.11 times faster than unthrottled, 8x16MB packfiles, 1 threads

In this case, the speedup is caused by us running git-index-pack(1) in
parallel. The improvement isn't linear, but still quite significant.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/fetch.adoc |  9 +++++
 fetch-pack.c                    | 86 +++++++++++++++++++++++++++++++++++++++--
 t/t5702-protocol-v2.sh          | 44 +++++++++++++++++++++
 3 files changed, 136 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index 00435e9a16..7afe8d7d5c 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -94,6 +94,15 @@ A value of 0 will give some reasonable default. If unset, it defaults to 1.
 For submodules, this setting can be overridden using the `submodule.fetchJobs`
 config setting.
 
+`fetch.packfileURIThreads`::
+	Specifies the number of threads used to download packfiles
+	advertised by the server via the `packfile-uris` capability in
+	parallel. Each packfile is downloaded via a separate
+	linkgit:git-http-fetch[1] process.
++
+A value of 0 will use a reasonable default based on the number of available
+CPUs. If unset, it defaults to 1, downloading packfiles sequentially.
+
 `fetch.writeCommitGraph`::
 	Set to true to write a commit-graph after every `git fetch` command
 	that downloads a pack-file from a remote. Using the `--split` option,
diff --git a/fetch-pack.c b/fetch-pack.c
index 6aca0b2588..b9dca9e07f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -37,6 +37,7 @@
 #include "mergesort.h"
 #include "prio-queue.h"
 #include "promisor-remote.h"
+#include "thread-utils.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -53,6 +54,7 @@ static struct shallow_lock shallow_lock;
 static const char *alternate_shallow_file;
 static struct strbuf fsck_msg_types = STRBUF_INIT;
 static struct string_list uri_protocols = STRING_LIST_INIT_DUP;
+static unsigned int packfile_uri_threads = 1;
 
 /* Remember to update object flag allocation in object.h */
 #define COMPLETE	(1U << 0)
@@ -1692,6 +1694,13 @@ static void fetch_packfile_uri(const char *uri_with_hash,
 	cmd.git_cmd = 1;
 	cmd.no_stdin = 1;
 	cmd.out = -1;
+
+	/*
+	 * Multiple threads may spawn and reap children concurrently in here.
+	 * This is safe because the child-cleanup bookkeeping in run-command.c,
+	 * which is not thread-safe, is only ever used when `clean_on_exit` is
+	 * set.
+	 */
 	if (start_command(&cmd))
 		die("fetch-pack: unable to spawn http-fetch");
 
@@ -1720,22 +1729,84 @@ static void fetch_packfile_uri(const char *uri_with_hash,
 		    uri_with_hash);
 }
 
+struct fetch_packfile_uris_state {
+	const struct string_list *packfile_uris;
+	const struct strvec *index_pack_args;
+	struct fetch_packfile_uri_result *results;
+	size_t next;
+	pthread_mutex_t lock;
+};
+
+static void *fetch_packfile_uris_thread(void *data)
+{
+	struct fetch_packfile_uris_state *state = data;
+
+	trace2_thread_start("fetch_packfile_uri");
+
+	for (;;) {
+		size_t i;
+
+		pthread_mutex_lock(&state->lock);
+		i = state->next++;
+		pthread_mutex_unlock(&state->lock);
+		if (i >= state->packfile_uris->nr)
+			break;
+
+		fetch_packfile_uri(state->packfile_uris->items[i].string,
+				   state->index_pack_args,
+				   &state->results[i]);
+	}
+
+	trace2_thread_exit();
+
+	return NULL;
+}
+
 static void fetch_packfile_uris(const struct string_list *packfile_uris,
 				const struct strvec *index_pack_args,
 				struct oidset *gitmodules_found,
 				struct string_list *pack_lockfiles)
 {
+	unsigned int nr_threads = packfile_uri_threads;
 	struct fetch_packfile_uri_result *results;
 
+	if (!nr_threads)
+		nr_threads = online_cpus();
+	if (nr_threads > packfile_uris->nr)
+		nr_threads = packfile_uris->nr;
+
 	/* Initialize the data. */
 	CALLOC_ARRAY(results, packfile_uris->nr);
 	for (size_t i = 0; i < packfile_uris->nr; i++)
 		oidset_init(&results[i].gitmodules_found, 0);
 
 	/* Perform the fetches. */
-	for (size_t i = 0; i < packfile_uris->nr; i++)
-		fetch_packfile_uri(packfile_uris->items[i].string,
-				   index_pack_args, &results[i]);
+	if (nr_threads > 1) {
+		struct fetch_packfile_uris_state state = {
+			.packfile_uris = packfile_uris,
+			.index_pack_args = index_pack_args,
+			.results = results,
+		};
+		pthread_t *threads;
+
+		pthread_mutex_init(&state.lock, NULL);
+		ALLOC_ARRAY(threads, nr_threads);
+
+		for (size_t i = 0; i < nr_threads; i++)
+			if (pthread_create(&threads[i], NULL,
+					   fetch_packfile_uris_thread, &state))
+				die(_("failed to create thread"));
+		for (size_t i = 0; i < nr_threads; i++)
+			if (pthread_join(threads[i], NULL))
+				die(_("failed to join thread"));
+
+		pthread_mutex_destroy(&state.lock);
+		free(threads);
+	} else {
+		for (size_t i = 0; i < packfile_uris->nr; i++)
+			fetch_packfile_uri(packfile_uris->items[i].string,
+					   index_pack_args, &results[i]);
+	}
 
 	/* Aggregate results. */
 	for (size_t i = 0; i < packfile_uris->nr; i++) {
@@ -2018,6 +2089,15 @@ static void fetch_pack_config(void)
 		}
 	}
 
+	if (!repo_config_get_uint(the_repository, "fetch.packfileurithreads",
+				  &packfile_uri_threads)) {
+		if (!HAVE_THREADS && packfile_uri_threads != 1) {
+			warning(_("no threads support, ignoring %s"),
+				"fetch.packfileURIThreads");
+			packfile_uri_threads = 1;
+		}
+	}
+
 	repo_config(the_repository, fetch_pack_config_cb, NULL);
 }
 
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 0f05286de8..a43d64ac95 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -1270,6 +1270,50 @@ test_expect_success 'part of packfile response provided as URI' '
 	test_line_count = 6 filelist
 '
 
+test_expect_success 'packfile URIs are downloaded in parallel' '
+	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+	rm -rf "$P" http_child log trace2.txt &&
+
+	git init "$P" &&
+	git -C "$P" config "uploadpack.allowsidebandall" "true" &&
+
+	for i in one two three
+	do
+		echo blob-$i >"$P"/blob-$i &&
+		git -C "$P" add blob-$i &&
+		configure_exclusion "$P" blob-$i >h-$i || return 1
+	done &&
+	git -C "$P" commit -m message &&
+
+	GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TEST_SIDEBAND_ALL=1 git \
+		-c protocol.version=2 \
+		-c fetch.uriprotocols=http,https \
+		-c fetch.packfileurithreads=2 \
+		clone --quiet "$HTTPD_URL/smart/http_parent" http_child 2>err &&
+
+	# Ensure that all objects were found.
+	for i in one two three
+	do
+		git -C http_child cat-file -e "$(cat h-$i)" || return 1
+	done &&
+
+	# Ensure that there are exactly 4 packfiles with associated .idx.
+	ls http_child/.git/objects/pack/*.pack \
+	    http_child/.git/objects/pack/*.idx >filelist &&
+	test_line_count = 8 filelist &&
+
+	if test_have_prereq PTHREADS
+	then
+		# Ensure that exactly two worker threads were spawned.
+		git grep --no-index --only-matching "\"thread\":\"th[0-9]*:fetch_packfile_uri\"" trace2.txt >threads &&
+		sort -u <threads >threads.unique &&
+		test_line_count = 2 threads.unique &&
+		test_grep ! "warning: no threads support, ignoring fetch.packfileURIThreads" err
+	else
+		test_grep "warning: no threads support, ignoring fetch.packfileURIThreads" err
+	fi
+'
+
 test_expect_success 'packfile URIs with fetch instead of clone' '
 	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
 	rm -rf "$P" http_child log &&

-- 
2.55.0.822.g20453c30eb.dirty

Re: [PATCH 0/2] fetch-pack: allow parallelizing packfile URI fetches

[Justin Tobler]

On 26/08/21 02:31PM, Patrick Steinhardt wrote:
> Hi,
> 
> this patch series prepares git-fetch(1) and git-clone(1) to handle
> fetches of packfile URIs in parallel. This can significantly speed up
> fetches when the server announces a bunch of packfiles, as shown in the
> benchmarks in the second patch.

So I've been working on a series to extend the use of the ODB
transaction interface to also cover fetch-pack. As part of this, my
current plan was to also refactor fetching packfile URIs so that they
can be written through `odb_transaction_write_pack()`. I like what this
series is doing, but I wonder if it might a bit more straightforward if
we try to land transaction here first. Otherwise, I think we may end up
having to redo some of these changes to get parallelizing packfile URI
fetches to work.

-Justin

Re: [PATCH 0/2] fetch-pack: allow parallelizing packfile URI fetches

[Patrick Steinhardt]

On Sun, Aug 30, 2026 at 09:33:52PM -0500, Justin Tobler wrote:
> On 26/08/21 02:31PM, Patrick Steinhardt wrote:
> > Hi,
> > 
> > this patch series prepares git-fetch(1) and git-clone(1) to handle
> > fetches of packfile URIs in parallel. This can significantly speed up
> > fetches when the server announces a bunch of packfiles, as shown in the
> > benchmarks in the second patch.
> 
> So I've been working on a series to extend the use of the ODB
> transaction interface to also cover fetch-pack. As part of this, my
> current plan was to also refactor fetching packfile URIs so that they
> can be written through `odb_transaction_write_pack()`. I like what this
> series is doing, but I wonder if it might a bit more straightforward if
> we try to land transaction here first. Otherwise, I think we may end up
> having to redo some of these changes to get parallelizing packfile URI
> fetches to work.

I'm fine to drop this series for now in favor of yours. I'll resend once
your series has been merged. Thanks!

Patrick