From b99b73a5e964f659e1c6279da29261c4dcc365ea Mon Sep 17 00:00:00 2001 From: Oleg Utkin Date: Wed, 26 Aug 2026 18:51:53 +0000 Subject: [PATCH] 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. --- Documentation/config/fetch.adoc | 12 ++- Documentation/config/pack.adoc | 5 + fetch-pack.c | 25 +++-- t/t5702-protocol-v2.sh | 171 ++++++++++++++++++++++++++++++++ 4 files changed, 204 insertions(+), 9 deletions(-) diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc index be055c35813d57..8bd2384b2f7653 100644 --- a/Documentation/config/fetch.adoc +++ b/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 diff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc index 22384c2d2f0773..def771bbb257cb 100644 --- a/Documentation/config/pack.adoc +++ b/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 diff --git a/fetch-pack.c b/fetch-pack.c index b6925c2238dfd2..5fb239840978fe 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; @@ -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); diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 0b87bf4779cd6f..32d656fc1fa455 100755 --- a/t/t5702-protocol-v2.sh +++ b/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 &&