fetch-pack: derive tunable worker defaults for URI packs

Packfile URI fetches default to one job, and parallel URI indexers force
one thread even when pack.threads is configured. Use online CPUs to
choose defaults while retaining the existing configuration controls.

Default fetch.packfileUriJobs to the online CPU count capped at eight.
Divide CPUs among the actual parallel jobs for each indexer, with a
one-thread minimum and four-thread automatic maximum. Explicit positive
pack.threads overrides that budget; explicit URI job values remain
uncapped and values at or below one retain serial processing.

Preserve no-ref-delta negotiation and verification, serial fallback,
pack ownership and cancellation. Document that increasing both controls
can increase CPU and memory use.

Verification: the new default-concurrency regression fails on the old
implementation and passes with the change. The developer build and all
102 t5702 protocol tests pass, including real HTTP coverage, explicit
thread values, job overrides and no-ref-delta validation. All 70
applicable t5300 index-pack tests pass; two !PTHREADS cases do not apply
to this threaded build. Chainlint, greplint and documentation style
checks pass. Independent review found no introduced P0/P1/P2 defects.

Author: uoleg-oai

Documentation/config/fetch.adoc

@@ -96,13 +96,21 @@ config setting.
 
 `fetch.packfileUriJobs`::
 	Specifies the maximum number of packfile URI downloads and indexers
-	to run at once. The default is 1, which preserves advertised URI
-	order.
+	to run at once. If unset, defaults to the number of online CPUs,
+	capped at 8. Values of 1 or less preserve advertised URI order.
+	An explicit value greater than 8 is permitted. The number of URI
+	packs in the response also limits the number of jobs.
 +
 Values greater than 1 are used only when the server advertises and the
 client requests the `no-ref-delta` promise. Each URI pack is then checked
 with `index-pack --no-ref-delta` before it is accepted.
 Responses with one URI retain the serial path.
++
+When `pack.threads` is unset or zero, each parallel URI indexer uses the
+number of online CPUs divided by the actual number of jobs, rounded down,
+with a minimum of 1 thread and a maximum of 4. A positive `pack.threads`
+value overrides this per-indexer default, including values greater than 4.
+Increasing both settings can increase CPU and memory use substantially.
 
 `fetch.writeCommitGraph`::
 	Set to true to write a commit-graph after every `git fetch` command

Documentation/config/pack.adoc

@@ -81,6 +81,11 @@ pack.threads::
 	is however multiplied by the number of threads.
 	Specifying 0 will cause Git to auto-detect the number of CPUs
 	and set the number of threads accordingly.
++
+This also sets the number of threads used to resolve deltas in
+linkgit:git-index-pack[1]. Parallel packfile URI indexers share the online
+CPUs when this is unset or zero; see `fetch.packfileUriJobs` for their
+automatic limits. A positive value applies to each indexer independently.
 
 pack.indexVersion::
 	Specify the default pack index version.  Valid values are 1 for

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;
@@ -49,7 +50,7 @@ static int fetch_fsck_objects = -1;
 static int transfer_fsck_objects = -1;
 static int agent_supported;
 static int server_supports_filtering;
-static int fetch_packfile_uri_jobs = 1;
+static int fetch_packfile_uri_jobs;
 static struct shallow_lock shallow_lock;
 static const char *alternate_shallow_file;
 static struct strbuf fsck_msg_types = STRBUF_INIT;
@@ -1778,10 +1779,23 @@ static void fetch_packfile_uris_parallel(
 	struct packfile_uri_task *tasks;
 	struct pollfd *pollfds;
 	int precreate_keeps = index_pack_args_have_keep(index_pack_args);
+	int threads = 0;
 	size_t next = 0, running = 0;
 
 	if (task_nr > (size_t)fetch_packfile_uri_jobs)
 		task_nr = fetch_packfile_uri_jobs;
+
+	/* Share CPUs across active packs unless the caller chose a thread count. */
+	repo_config_get_int(the_repository, "pack.threads", &threads);
+	if (!threads) {
+		threads = online_cpus() / (int)task_nr;
+		if (threads < 1)
+			threads = 1;
+		else if (threads > 4)
+			threads = 4;
+	}
+	strvec_pushf(index_pack_args, "--threads=%d", threads);
+
 	CALLOC_ARRAY(tasks, task_nr);
 	CALLOC_ARRAY(pollfds, task_nr);
 
@@ -2036,12 +2050,6 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 		fetch_packfile_uri_jobs > 1 &&
 		packfile_uris.nr > 1;
 	if (parallel_uri_indexing) {
-		/*
-		 * Bound total indexer threads by the number of URI jobs. The
-		 * URI packs are independent, so each indexer can stay single
-		 * threaded while several indexers run at once.
-		 */
-		strvec_push(&index_pack_args, "--threads=1");
 		fetch_packfile_uris_parallel(&packfile_uris, &index_pack_args,
 					    pack_lockfiles,
 					    &fsck_options.gitmodules_found);
@@ -2166,6 +2174,9 @@ static int fetch_pack_config_cb(const char *var, const char *value,
 
 static void fetch_pack_config(void)
 {
+	fetch_packfile_uri_jobs = online_cpus();
+	if (fetch_packfile_uri_jobs > 8)
+		fetch_packfile_uri_jobs = 8;
 	repo_config_get_int(the_repository, "fetch.unpacklimit", &fetch_unpack_limit);
 	repo_config_get_int(the_repository, "transfer.unpacklimit", &transfer_unpack_limit);
 	repo_config_get_bool(the_repository, "repack.usedeltabaseoffset", &prefer_ofs_delta);

t/t5702-protocol-v2.sh

@@ -1321,6 +1321,7 @@ test_expect_success 'no-ref-delta URI packs are indexed concurrently' '
 	GIT_TEST_SIDEBAND_ALL=1 \
 	git -c protocol.version=2 -c fetch.uriprotocols=http \
 		-c fetch.packfileUriJobs=2 \
+		-c pack.threads=1 \
 		clone "$HTTPD_URL/smart/http_parent" http_child-no-ref &&
 
 	test_grep "> no-ref-delta" no-ref-packet.trace &&
@@ -1332,6 +1333,176 @@ test_expect_success 'no-ref-delta URI packs are indexed concurrently' '
 	test_line_count = 3 no-ref-uri-indexers
 '
 
+test_expect_success 'setup packfile URI worker tuning' '
+	P="$HTTPD_DOCUMENT_ROOT_PATH/http_tuning" &&
+	git init "$P" &&
+	git -C "$P" config uploadpack.allowsidebandall true &&
+	git -C "$P" config uploadpack.allowNoRefDelta true &&
+	for i in $(test_seq 10)
+	do
+		echo "$i" >"$P/blob-$i" &&
+		git -C "$P" add "blob-$i" || return 1
+	done &&
+	git -C "$P" commit -m objects &&
+	for i in $(test_seq 10)
+	do
+		configure_exclusion "$P" "blob-$i" >/dev/null || return 1
+	done &&
+	uri_cpus=$(test-tool online-cpus)
+'
+
+clone_uri_tuning () {
+	local repository="$1" destination="$2" &&
+	shift 2 &&
+	GIT_TRACE2_EVENT="$TRASH_DIRECTORY/$destination.trace" \
+	GIT_TRACE_PACKET="$TRASH_DIRECTORY/$destination.packet" \
+	GIT_TEST_SIDEBAND_ALL=1 \
+	git -c protocol.version=2 -c fetch.uriprotocols=http "$@" \
+		clone "$HTTPD_URL/smart/$repository" "$destination" &&
+	git -C "$destination" fsck --no-reflogs
+}
+
+# Count outstanding URI children in their parent's Trace2 events. Indexers
+# have their own child IDs, so pair each ID with its parent's session ID.
+test_uri_fetch_jobs () {
+	awk -v expected="$2" '
+	function child_key(    fields, sid, id) {
+		split($0, fields, "\"sid\":\"");
+		split(fields[2], sid, "\"");
+		split($0, fields, "\"child_id\":");
+		id = fields[2] + 0;
+		return sid[1] ":" id;
+	}
+	/"event":"child_start".*"http-fetch","--packfile=/ {
+		active[child_key()] = 1;
+		running++;
+		if (running > maximum)
+			maximum = running;
+	}
+	/"event":"child_exit"/ {
+		key = child_key();
+		if (active[key]) {
+			delete active[key];
+			running--;
+		}
+	}
+	END {
+		if (running != 0 || maximum != expected) {
+			printf "URI workers: maximum %d, expected %d, outstanding %d\n",
+				maximum, expected, running;
+			exit 1;
+		}
+	}
+	' "$1"
+}
+
+test_expect_success 'packfile URI defaults bound workers and indexer threads' '
+	jobs=$uri_cpus &&
+	if test "$jobs" -gt 8
+	then
+		jobs=8
+	fi &&
+	clone_uri_tuning http_tuning uri-default &&
+	test_uri_fetch_jobs uri-default.trace "$jobs" &&
+	if test "$jobs" -gt 1
+	then
+		threads=$((uri_cpus / jobs)) &&
+		if test "$threads" -gt 4
+		then
+			threads=4
+		fi &&
+		test_grep "> no-ref-delta" uri-default.packet &&
+		grep "\"event\":\"child_start\".*\"index-pack\".*--no-ref-delta.*--threads=$threads\"" \
+			uri-default.trace >uri-default.indexers &&
+		test_line_count = 10 uri-default.indexers
+	else
+		test_grep ! "> no-ref-delta" uri-default.packet
+	fi
+'
+
+for threads in 1 2 4 8
+do
+	test_expect_success "parallel URI indexers honor pack.threads=$threads" '
+		clone_uri_tuning http_tuning uri-threads-$threads \
+			-c fetch.packfileUriJobs=2 -c pack.threads=$threads &&
+		test_uri_fetch_jobs uri-threads-$threads.trace 2 &&
+		grep "\"event\":\"child_start\".*\"index-pack\".*--no-ref-delta.*--threads=$threads\"" \
+			uri-threads-$threads.trace >uri-indexers &&
+		test_line_count = 10 uri-indexers
+	'
+done
+
+test_expect_success 'automatic URI threads use the actual worker count' '
+	# More requested jobs than URI packs must not dilute the CPU budget.
+	threads=$((uri_cpus / 3)) &&
+	if test "$threads" -lt 1
+	then
+		threads=1
+	elif test "$threads" -gt 4
+	then
+		threads=4
+	fi &&
+	clone_uri_tuning http_parent uri-auto-threads \
+		-c fetch.packfileUriJobs=100 -c pack.threads=0 &&
+	test_uri_fetch_jobs uri-auto-threads.trace 3 &&
+	grep "\"event\":\"child_start\".*\"index-pack\".*--no-ref-delta.*--threads=$threads\"" \
+		uri-auto-threads.trace >uri-indexers &&
+	test_line_count = 3 uri-indexers
+'
+
+test_expect_success 'explicit URI jobs can exceed the automatic limit' '
+	clone_uri_tuning http_tuning uri-many-jobs \
+		-c fetch.packfileUriJobs=100 -c pack.threads=1 &&
+	test_uri_fetch_jobs uri-many-jobs.trace 10
+'
+
+test_expect_success 'fetch honors packfile URI worker tuning' '
+	git init uri-fetch &&
+	GIT_TRACE2_EVENT="$TRASH_DIRECTORY/uri-fetch.trace" \
+	GIT_TEST_SIDEBAND_ALL=1 \
+	git -C uri-fetch -c protocol.version=2 -c fetch.uriprotocols=http \
+		-c fetch.packfileUriJobs=2 -c pack.threads=2 \
+		fetch "$HTTPD_URL/smart/http_tuning" &&
+	test_uri_fetch_jobs uri-fetch.trace 2 &&
+	grep "\"event\":\"child_start\".*\"index-pack\".*--no-ref-delta.*--threads=2\"" \
+		uri-fetch.trace >uri-indexers &&
+	test_line_count = 10 uri-indexers &&
+	git -C uri-fetch fsck --no-reflogs
+'
+
+for jobs in 0 1 -1
+do
+	test_expect_success "fetch.packfileUriJobs=$jobs retains serial indexing" '
+		clone_uri_tuning http_tuning uri-serial-$jobs \
+			-c fetch.packfileUriJobs=$jobs -c pack.threads=2 &&
+		test_uri_fetch_jobs uri-serial-$jobs.trace 1 &&
+		test_grep ! "> no-ref-delta" uri-serial-$jobs.packet &&
+		test_grep ! -e "--threads=" uri-serial-$jobs.trace
+	'
+done
+
+test_expect_success 'parallel URI defaults require the server promise' '
+	test_config -C "$P" uploadpack.allowNoRefDelta false &&
+	clone_uri_tuning http_tuning uri-no-promise &&
+	test_uri_fetch_jobs uri-no-promise.trace 1 &&
+	test_grep ! "> no-ref-delta" uri-no-promise.packet &&
+	test_grep ! -e "--threads=" uri-no-promise.trace
+'
+
+test_expect_success 'single URI retains ordinary indexer thread selection' '
+	P="$HTTPD_DOCUMENT_ROOT_PATH/http_single_uri" &&
+	git init "$P" &&
+	git -C "$P" config uploadpack.allowsidebandall true &&
+	git -C "$P" config uploadpack.allowNoRefDelta true &&
+	test_commit -C "$P" one &&
+	configure_exclusion "$P" one.t >/dev/null &&
+	clone_uri_tuning http_single_uri uri-single \
+		-c fetch.packfileUriJobs=8 -c pack.threads=2 &&
+	test_uri_fetch_jobs uri-single.trace 1 &&
+	test_grep "> no-ref-delta" uri-single.packet &&
+	test_grep ! -e "--threads=" uri-single.trace
+'
+
 test_expect_success 'packfile URIs with fetch instead of clone' '
 	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
 	rm -rf "$P" http_child log &&