From 633e355546f268474504340a8cd494c7eebf4c76 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 21 Aug 2026 15:06:22 -0500 Subject: [PATCH] index-pack: hash full blobs in a bounded worker pool The first pass through a pack inflates and hashes each object on the main thread. `pack.threads` applies later while resolving deltas, so SHA1DC work for full blobs remains serial even when CPUs are idle. In a 99 Hz profile of an 844,020,252-byte pack dominated by full blobs, SHA1DC accounted for 72.14% of sampled user CPU. Add an opt-in worker pool for complete heap-backed blobs. The producer continues parsing, inflating, computing CRCs, and writing the pack. The workers use the normal object hashing backend, including SHA1DC, while the main thread performs the existing ODB, collision, and content checks as deferred results are retired in queue order. Bound retained data by both bytes and job count. Strict, fsck, promisor, non-threaded, streamed, and otherwise ineligible objects retain the serial path. The default is disabled. Across three runs of the original prototype on that pack, two workers reduced median wall time from 36.076 to 19.227 seconds. CPU changed from 50.121 to 50.955 seconds and peak RSS from 413.7 to 409.8 MiB. The object/delta-heavy control did not show a wall-time improvement. The fixed-size queue used here has not been rebenchmarked. t5352 covers output equivalence, queue limits, serial fallbacks, collision and duplicate handling, REF/OFS ordering, corrupt input, and configuration validation. p5352 compares serial, one-worker, and two-worker indexing on a reproducible full-blob pack. Existing SHA-1 and SHA-256 index-pack tests also pass. --- Documentation/config.adoc | 2 + Documentation/config/indexpack.adoc | 21 ++ Documentation/git-index-pack.adoc | 5 + builtin/index-pack.c | 238 ++++++++++++++++++++++- t/meson.build | 2 + t/perf/p5352-index-pack-hash-pipeline.sh | 49 +++++ t/t5352-index-pack-hash-pipeline.sh | 177 +++++++++++++++++ 7 files changed, 490 insertions(+), 4 deletions(-) create mode 100644 Documentation/config/indexpack.adoc create mode 100755 t/perf/p5352-index-pack-hash-pipeline.sh create mode 100755 t/t5352-index-pack-hash-pipeline.sh diff --git a/Documentation/config.adoc b/Documentation/config.adoc index 1ef72de62f2ba6..a6b83c014288cd 100644 --- a/Documentation/config.adoc +++ b/Documentation/config.adoc @@ -518,6 +518,8 @@ include::config/includeif.adoc[] include::config/index.adoc[] +include::config/indexpack.adoc[] + include::config/init.adoc[] include::config/instaweb.adoc[] diff --git a/Documentation/config/indexpack.adoc b/Documentation/config/indexpack.adoc new file mode 100644 index 00000000000000..a56beccd390f1a --- /dev/null +++ b/Documentation/config/indexpack.adoc @@ -0,0 +1,21 @@ +indexPack.hashThreads:: + Experimental number of workers used to hash full blobs during the + first pass of linkgit:git-index-pack[1]. The default, zero, disables + the workers. Values from 1 through 32 are accepted. Parsing and + inflation remain serial, and normal collision/content validation runs + on the main thread. This is independent of `pack.threads` and of + concurrent packfile-URI downloads. Strict, fsck, and promisor modes, + and builds without thread support, use the existing serial path. + +indexPack.hashBufferSize:: + Maximum retained blob-buffer bytes for `indexPack.hashThreads`, + including the terminating byte, the producer's reserved buffer, and + completed results awaiting validation. Defaults to 64 MiB. At most + twice the number of hash workers can be outstanding. Objects that do + not fit use the existing serial path; the limit is not a bound on + Git's total memory use. Usual `k`, `m`, and `g` suffixes are accepted. + +indexPack.hashMinSize:: + Minimum full-blob size eligible for `indexPack.hashThreads`. + Defaults to 64 KiB. Blobs above `core.bigFileThreshold` retain their + existing streaming path. Usual `k`, `m`, and `g` suffixes are accepted. diff --git a/Documentation/git-index-pack.adoc b/Documentation/git-index-pack.adoc index 18036953c06b22..0cf251ac9bb65d 100644 --- a/Documentation/git-index-pack.adoc +++ b/Documentation/git-index-pack.adoc @@ -148,6 +148,11 @@ accessible through promisor objects. + Requires to not be specified. +CONFIGURATION +------------- + +include::config/indexpack.adoc[] + NOTES ----- diff --git a/builtin/index-pack.c b/builtin/index-pack.c index bc86925ad04340..9b6a0abbb2b44f 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -31,6 +31,7 @@ #include "run-command.h" #include "setup.h" #include "strvec.h" +#include "trace2.h" static const char index_pack_usage[] = "git index-pack [-v] [-o ] [--keep | --keep=] [--[no-]rev-index] [--verify] [--strict[==...]] [--fsck-objects[==...]] ( | --stdin [--fix-thin] [])"; @@ -43,6 +44,43 @@ struct object_entry { signed char real_type; }; +/* + * Hash workers own no repository state. They only hash immutable full-blob + * buffers; the main thread retires their results and performs the usual + * collision/content checks before releasing those buffers. + */ +struct first_pass_hash_job { + struct object_entry *obj; + struct object_id oid; + void *data; + int done; +}; + +struct first_pass_hash_pool { + pthread_t *threads; + pthread_mutex_t mutex; + pthread_cond_t work_ready; + pthread_cond_t result_ready; + struct first_pass_hash_job *queue; + size_t nr_threads, queue_size; + size_t first, next_work, nr, pending; + size_t buffered, jobs; + int stop; +}; + +#define FIRST_PASS_HASH_MAX_THREADS 32 + +static int first_pass_hash_threads; +static size_t first_pass_hash_buffer_size = 64 * 1024 * 1024; +static size_t first_pass_hash_min_size = 64 * 1024; +static struct first_pass_hash_pool first_pass_hash_pool; + +static struct first_pass_hash_job *reserve_first_pass_hash(struct object_entry *obj); +static void submit_first_pass_hash(struct first_pass_hash_job *job, + struct object_entry *obj, void *data); +static void start_first_pass_hash(void); +static void finish_first_pass_hash(void); + struct object_stat { unsigned delta_depth; int base_object_no; @@ -479,7 +517,7 @@ static void *unpack_entry_data(off_t offset, size_t size, char hdr[32]; int hdrlen; - if (!is_delta_type(type)) { + if (!is_delta_type(type) && oid) { hdrlen = format_object_header(hdr, sizeof(hdr), type, size); git_hash_init(&c, the_hash_algo); git_hash_update(&c, hdr, hdrlen); @@ -520,7 +558,8 @@ static void *unpack_entry_data(off_t offset, size_t size, static void *unpack_raw_entry(struct object_entry *obj, off_t *ofs_offset, struct object_id *ref_oid, - struct object_id *oid) + struct object_id *oid, + struct first_pass_hash_job **hash_job) { unsigned char *p; size_t size, c; @@ -582,7 +621,9 @@ static void *unpack_raw_entry(struct object_entry *obj, } obj->hdr_size = consumed_bytes - obj->idx.offset; - data = unpack_entry_data(obj->idx.offset, obj->size, obj->type, oid); + *hash_job = reserve_first_pass_hash(obj); + data = unpack_entry_data(obj->idx.offset, obj->size, obj->type, + *hash_job ? NULL : oid); obj->idx.crc32 = input_crc32; return data; } @@ -978,6 +1019,171 @@ static void sha1_object(const void *data, struct object_entry *obj_entry, free(new_data); } +static void *first_pass_hash_worker(void *data UNUSED) +{ + struct first_pass_hash_pool *pool = &first_pass_hash_pool; + + trace2_thread_start("index-pack-hash"); + for (;;) { + struct first_pass_hash_job *job; + + pthread_mutex_lock(&pool->mutex); + while (!pool->pending && !pool->stop) + pthread_cond_wait(&pool->work_ready, &pool->mutex); + if (!pool->pending) { + pthread_mutex_unlock(&pool->mutex); + break; + } + job = &pool->queue[pool->next_work]; + pool->next_work = (pool->next_work + 1) % pool->queue_size; + pool->pending--; + pthread_mutex_unlock(&pool->mutex); + + /* This uses the same collision-detecting hash as the serial path. */ + hash_object_file(the_hash_algo, job->data, job->obj->size, + OBJ_BLOB, &job->oid); + + pthread_mutex_lock(&pool->mutex); + job->done = 1; + pthread_cond_signal(&pool->result_ready); + pthread_mutex_unlock(&pool->mutex); + } + trace2_thread_exit(); + return NULL; +} + +static void retire_first_pass_hash(void) +{ + struct first_pass_hash_pool *pool = &first_pass_hash_pool; + struct first_pass_hash_job *job; + size_t allocation; + + pthread_mutex_lock(&pool->mutex); + if (!pool->nr) + BUG("no queued first-pass hash to retire"); + job = &pool->queue[pool->first]; + while (!job->done) + pthread_cond_wait(&pool->result_ready, &pool->mutex); + pool->first = (pool->first + 1) % pool->queue_size; + pool->nr--; + pthread_mutex_unlock(&pool->mutex); + + /* All ODB and object-cache access stays on the main thread. */ + oidcpy(&job->obj->idx.oid, &job->oid); + sha1_object(job->data, NULL, job->obj->size, OBJ_BLOB, + &job->obj->idx.oid); + allocation = st_add(job->obj->size, 1); + free(job->data); + pool->buffered -= allocation; +} + +static struct first_pass_hash_job *reserve_first_pass_hash(struct object_entry *obj) +{ + struct first_pass_hash_pool *pool = &first_pass_hash_pool; + size_t allocation; + + if (!pool->nr_threads || obj->type != OBJ_BLOB || + obj->size < first_pass_hash_min_size || + obj->size >= first_pass_hash_buffer_size || + obj->size > repo_settings_get_big_file_threshold(the_repository)) + return NULL; + + allocation = st_add(obj->size, 1); + while (pool->nr == pool->queue_size || + allocation > first_pass_hash_buffer_size - pool->buffered) + retire_first_pass_hash(); + + /* Reserve before the producer allocates the inflated blob. */ + pool->buffered += allocation; + return &pool->queue[(pool->first + pool->nr) % pool->queue_size]; +} + +static void submit_first_pass_hash(struct first_pass_hash_job *job, + struct object_entry *obj, void *data) +{ + struct first_pass_hash_pool *pool = &first_pass_hash_pool; + + assert(pool->nr_threads && data && obj->type == OBJ_BLOB); + job->obj = obj; + job->data = data; + job->done = 0; + + pthread_mutex_lock(&pool->mutex); + pool->nr++; + pool->pending++; + pool->jobs++; + pthread_cond_signal(&pool->work_ready); + pthread_mutex_unlock(&pool->mutex); +} + +static void stop_first_pass_hash(size_t nr_threads) +{ + struct first_pass_hash_pool *pool = &first_pass_hash_pool; + size_t i; + + pthread_mutex_lock(&pool->mutex); + pool->stop = 1; + pthread_cond_broadcast(&pool->work_ready); + pthread_mutex_unlock(&pool->mutex); + for (i = 0; i < nr_threads; i++) + pthread_join(pool->threads[i], NULL); + pthread_cond_destroy(&pool->result_ready); + pthread_cond_destroy(&pool->work_ready); + pthread_mutex_destroy(&pool->mutex); + free(pool->queue); + free(pool->threads); +} + +static void start_first_pass_hash(void) +{ + struct first_pass_hash_pool *pool = &first_pass_hash_pool; + size_t i; + int ret; + + /* These modes share fsck/object-cache state; retain their serial path. */ + if (!HAVE_THREADS || !first_pass_hash_threads || strict || + do_fsck_object || record_outgoing_links || + first_pass_hash_min_size >= first_pass_hash_buffer_size) { + trace2_data_intmax("index-pack", the_repository, + "first_pass_hash/threads", 0); + return; + } + + pool->nr_threads = first_pass_hash_threads; + pool->queue_size = st_mult(pool->nr_threads, 2); + CALLOC_ARRAY(pool->threads, pool->nr_threads); + CALLOC_ARRAY(pool->queue, pool->queue_size); + pthread_mutex_init(&pool->mutex, NULL); + pthread_cond_init(&pool->work_ready, NULL); + pthread_cond_init(&pool->result_ready, NULL); + for (i = 0; i < pool->nr_threads; i++) { + ret = pthread_create(&pool->threads[i], NULL, + first_pass_hash_worker, NULL); + if (ret) { + stop_first_pass_hash(i); + die(_("unable to create index-pack hash thread: %s"), + strerror(ret)); + } + } + trace2_data_intmax("index-pack", the_repository, + "first_pass_hash/threads", pool->nr_threads); +} + +static void finish_first_pass_hash(void) +{ + struct first_pass_hash_pool *pool = &first_pass_hash_pool; + + if (!pool->nr_threads) + return; + while (pool->nr) + retire_first_pass_hash(); + + stop_first_pass_hash(pool->nr_threads); + trace2_data_intmax("index-pack", the_repository, + "first_pass_hash/jobs", pool->jobs); + pool->nr_threads = 0; +} + /* * Ensure that this node has been reconstructed and return its contents. * @@ -1255,6 +1461,8 @@ static void parse_pack_objects(unsigned char *hash) struct stat st; struct git_hash_ctx tmp_ctx; + start_first_pass_hash(); + if (verbose) progress = start_progress( the_repository, @@ -1263,9 +1471,10 @@ static void parse_pack_objects(unsigned char *hash) nr_objects); for (i = 0; i < nr_objects; i++) { struct object_entry *obj = &objects[i]; + struct first_pass_hash_job *hash_job; void *data = unpack_raw_entry(obj, &ofs_delta->offset, &ref_delta_oid, - &obj->idx.oid); + &obj->idx.oid, &hash_job); obj->real_type = obj->type; if (obj->type == OBJ_OFS_DELTA) { nr_ofs_deltas++; @@ -1276,6 +1485,9 @@ static void parse_pack_objects(unsigned char *hash) oidcpy(&ref_deltas[nr_ref_deltas].oid, &ref_delta_oid); ref_deltas[nr_ref_deltas].obj_no = i; nr_ref_deltas++; + } else if (hash_job) { + submit_first_pass_hash(hash_job, obj, data); + data = NULL; } else if (!data) { /* large blobs, check later */ obj->real_type = OBJ_BAD; @@ -1287,6 +1499,7 @@ static void parse_pack_objects(unsigned char *hash) display_progress(progress, i+1); } objects[i].idx.offset = consumed_bytes; + finish_first_pass_hash(); stop_progress(&progress); /* Check pack integrity */ @@ -1667,6 +1880,23 @@ static int git_index_pack_config(const char *k, const char *v, { struct pack_idx_option *opts = cb; + if (!strcmp(k, "indexpack.hashthreads")) { + first_pass_hash_threads = git_config_int(k, v, ctx->kvi); + if (first_pass_hash_threads < 0 || + first_pass_hash_threads > FIRST_PASS_HASH_MAX_THREADS) + die(_("%s must be between 0 and %d"), + k, FIRST_PASS_HASH_MAX_THREADS); + return 0; + } + if (!strcmp(k, "indexpack.hashbuffersize")) { + first_pass_hash_buffer_size = git_config_ulong(k, v, ctx->kvi); + return 0; + } + if (!strcmp(k, "indexpack.hashminsize")) { + first_pass_hash_min_size = git_config_ulong(k, v, ctx->kvi); + return 0; + } + if (!strcmp(k, "pack.indexversion")) { opts->version = git_config_int(k, v, ctx->kvi); if (opts->version > 2) diff --git a/t/meson.build b/t/meson.build index a25f37d2f5ae7d..65e26a371bd8f3 100644 --- a/t/meson.build +++ b/t/meson.build @@ -639,6 +639,7 @@ integration_tests = [ 't5334-incremental-multi-pack-index.sh', 't5335-compact-multi-pack-index.sh', 't5351-unpack-large-objects.sh', + 't5352-index-pack-hash-pipeline.sh', 't5400-send-pack.sh', 't5401-update-hooks.sh', 't5402-post-merge-hook.sh', @@ -1172,6 +1173,7 @@ benchmarks = [ 'perf/p5326-multi-pack-bitmaps.sh', 'perf/p5332-multi-pack-reuse.sh', 'perf/p5333-pseudo-merge-bitmaps.sh', + 'perf/p5352-index-pack-hash-pipeline.sh', 'perf/p5550-fetch-tags.sh', 'perf/p5551-fetch-rescan.sh', 'perf/p5600-partial-clone.sh', diff --git a/t/perf/p5352-index-pack-hash-pipeline.sh b/t/perf/p5352-index-pack-hash-pipeline.sh new file mode 100755 index 00000000000000..c9af7eafc5e6da --- /dev/null +++ b/t/perf/p5352-index-pack-hash-pipeline.sh @@ -0,0 +1,49 @@ +#!/bin/sh + +test_description='Test index-pack first-pass hash worker performance + +GIT_PERF_5352_NR_BLOBS controls the number of full blobs in the input pack. +GIT_PERF_5352_BLOB_SIZE controls the size of each blob in bytes. +Keep the blob size between 65536 and 67108863 to exercise the worker path. +' + +. ./perf-lib.sh + +test_perf_fresh_repo + +: ${GIT_PERF_5352_NR_BLOBS:=128} +: ${GIT_PERF_5352_BLOB_SIZE:=1048576} + +test_expect_success 'create a pack of full blobs' ' + for i in $(test_seq 1 "$GIT_PERF_5352_NR_BLOBS") + do + test-tool genrandom "index-pack-hash-$i" \ + "$GIT_PERF_5352_BLOB_SIZE" | + git hash-object -w --stdin || return 1 + done >oids && + git pack-objects --stdout --window=0 input.pack +' + +test_size 'pack size' ' + test_file_size input.pack +' + +test_perf 'index-pack, serial hash' \ + --setup 'rm -rf repo.git && git init --bare -q repo.git' ' + git -C repo.git -c indexPack.hashThreads=0 \ + index-pack --threads=1 --stdin /dev/null +' + +test_perf 'index-pack, one hash worker' --prereq PTHREADS \ + --setup 'rm -rf repo.git && git init --bare -q repo.git' ' + git -C repo.git -c indexPack.hashThreads=1 \ + index-pack --threads=1 --stdin /dev/null +' + +test_perf 'index-pack, two hash workers' --prereq PTHREADS \ + --setup 'rm -rf repo.git && git init --bare -q repo.git' ' + git -C repo.git -c indexPack.hashThreads=2 \ + index-pack --threads=1 --stdin /dev/null +' + +test_done diff --git a/t/t5352-index-pack-hash-pipeline.sh b/t/t5352-index-pack-hash-pipeline.sh new file mode 100755 index 00000000000000..6b1642c2a7f561 --- /dev/null +++ b/t/t5352-index-pack-hash-pipeline.sh @@ -0,0 +1,177 @@ +#!/bin/sh + +test_description='bounded first-pass index-pack hash workers' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-pack.sh + +run_index () { + name=$1 && + input=$2 && + shift 2 && + git init --bare "$name.git" && + GIT_TRACE2_EVENT="$TRASH_DIRECTORY/$name.trace" \ + git -C "$name.git" "$@" <"$input" >"$name.out" +} + +trace_value () { + key=$1 && + value=$2 && + file=$3 && + test_grep "\"key\":\"first_pass_hash/$key\",\"value\":\"$value\"" "$file" +} + +compare_pack_files () { + left=$1 && + right=$2 && + pack_hash=$(cut -f2 "$left.out") && + test_cmp "$left.out" "$right.out" && + for suffix in pack idx rev + do + test_cmp_bin "$left.git/objects/pack/pack-$pack_hash.$suffix" \ + "$right.git/objects/pack/pack-$pack_hash.$suffix" || return 1 + done +} + +test_expect_success 'make a pack of full blobs' ' + for i in $(test_seq 1 8) + do + test-tool genrandom "hash-pipeline-$i" 65536 >"blob-$i" && + git hash-object -w "blob-$i" || return 1 + done >oids && + git pack-objects --stdout --window=0 input.pack && + run_index serial input.pack index-pack --stdin --fix-thin && + trace_value threads 0 serial.trace +' + +test_expect_success PTHREADS 'workers preserve pack, index and reverse index' ' + run_index parallel input.pack \ + -c indexPack.hashThreads=2 \ + -c indexPack.hashBufferSize=131074 \ + index-pack --stdin --fix-thin && + compare_pack_files serial parallel && + trace_value threads 2 parallel.trace && + trace_value jobs 8 parallel.trace +' + +test_expect_success PTHREADS 'an object must fit including its trailing byte' ' + run_index too-small input.pack \ + -c indexPack.hashThreads=2 \ + -c indexPack.hashBufferSize=65536 \ + -c indexPack.hashMinSize=0 \ + index-pack --stdin --fix-thin && + compare_pack_files serial too-small && + trace_value jobs 0 too-small.trace +' + +test_expect_success PTHREADS 'streamed large blobs retain the serial path' ' + run_index streamed input.pack \ + -c indexPack.hashThreads=2 \ + -c indexPack.hashMinSize=0 \ + -c core.bigFileThreshold=1 \ + index-pack --stdin --fix-thin && + compare_pack_files serial streamed && + trace_value jobs 0 streamed.trace +' + +test_expect_success PTHREADS 'validation modes retain the serial path' ' + for mode in strict fsck-objects promisor + do + run_index "$mode" input.pack \ + -c indexPack.hashThreads=2 \ + -c indexPack.hashMinSize=0 \ + index-pack --stdin --fix-thin "--$mode" && + trace_value threads 0 "$mode.trace" || return 1 + done +' + +test_expect_success PTHREADS 'existing-object collision check is not skipped' ' + git init --bare collision.git && + a=$(git -C collision.git hash-object -w ../blob-1) && + b=$(git -C collision.git hash-object -w ../blob-2) && + a_path=$(echo "$a" | sed "s!^..!&/!") && + b_path=$(echo "$b" | sed "s!^..!&/!") && + cp -f "collision.git/objects/$b_path" "collision.git/objects/$a_path" && + test_env GIT_TRACE2_EVENT="$TRASH_DIRECTORY/collision.trace" \ + test_must_fail git -C collision.git \ + -c indexPack.hashThreads=2 \ + index-pack --stdin --fix-thin collision.err && + trace_value threads 2 collision.trace && + test_grep "SHA1 COLLISION FOUND" collision.err +' + +test_expect_success PTHREADS 'duplicate full blobs preserve native acceptance' ' + { + pack_header 2 && + pack_obj "$EMPTY_BLOB" && + pack_obj "$EMPTY_BLOB" + } >duplicates.pack && + pack_trailer duplicates.pack && + run_index duplicate-serial duplicates.pack index-pack --stdin && + run_index duplicate-parallel duplicates.pack \ + -c indexPack.hashThreads=2 \ + -c indexPack.hashBufferSize=2 \ + -c indexPack.hashMinSize=0 \ + index-pack --stdin && + compare_pack_files duplicate-serial duplicate-parallel && + trace_value jobs 2 duplicate-parallel.trace && + git init --bare duplicate-strict.git && + test_must_fail git -C duplicate-strict.git \ + -c indexPack.hashThreads=2 \ + -c indexPack.hashMinSize=0 \ + index-pack --strict --stdin ref.pack && + pack_trailer ref.pack && + pack_obj "$A" "$B" >ref-entry && + { + pack_header 2 && + pack_obj "$B" && + # The full B entry is eleven bytes; the delta data is five. + printf "\145\013" && + dd if=ref-entry bs=1 skip=$((1 + $(test_oid rawsz))) + } >ofs.pack && + pack_trailer ofs.pack && + for kind in ref ofs + do + run_index "$kind" "$kind.pack" \ + -c indexPack.hashThreads=2 \ + -c indexPack.hashMinSize=0 \ + index-pack --stdin --fix-thin && + trace_value jobs 1 "$kind.trace" && + git -C "$kind.git" cat-file blob "$A" >actual && + test "$(git hash-object actual)" = "$A" || return 1 + done +' + +test_expect_success PTHREADS 'bad input cannot publish an index' ' + length=$(wc -c actual && + test_must_be_empty actual || return 1 + done +' + +test_expect_success 'invalid worker counts are rejected' ' + test_must_fail git -c indexPack.hashThreads=-1 index-pack input.pack && + test_must_fail git -c indexPack.hashThreads=33 index-pack input.pack +' + +test_done