{"thread":{"id":"66103","subject":"[PATCH] index-pack: speed up promisor link recording","startedAt":"2026-08-02T21:33:19Z","lastAt":"2026-08-03T00:55:18Z","messageCount":9,"participants":["Arijit Banerjee via GitGitGadget","brian m. carlson","Arijit Banerjee","Junio C Hamano","Collin Funk"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"549449","messageId":"pull.2191.git.1785706396130.gitgitgadget@gmail.com","threadId":"66103","inReplyTo":null,"subject":"[PATCH] index-pack: speed up promisor link recording","fromName":"Arijit Banerjee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-02T21:33:15Z","receivedAt":"2026-08-02T21:33:19Z","isPatch":true,"body":"From: Arijit Banerjee <arijit@effectiveailabs.com>\n\nWhen indexing a promisor pack, index-pack parses every reconstructed\nnon-blob object into the shared object model to record its outgoing links.\nSince parse_object_buffer() runs under read_mutex, worker threads serialize\nwhile allocating persistent tree, commit, and tag structures that are only\nneeded to enumerate those links.\n\nRead the links directly from the reconstructed object buffers instead. Keep\nthe strict and fsck paths unchanged, use worker-local typed oidmaps during\nnormal promisor indexing, and merge them after the workers exit. Transfer\nentries during the merge so that it does not temporarily duplicate the\ncomplete link set.\n\nThe typed entries preserve checks previously performed as a side effect of\nobject parsing. Reject malformed commit and tag headers, conflicting\nexpected types, and targets whose actual type disagrees when the target is\npresent in the pack. Preserve commit-graft handling and the existing policy\nof recording only subtree entries from trees.\n\nWith three runs per version on Debian 12, median end-to-end wall-clock time\nfor a --filter=blob:none clone of linux.git decreased from 156 seconds to\n133 seconds (15%). Trace2 attributed the change to the initial index-pack\n--promisor phase, whose median duration decreased from 121 seconds to 98\nseconds (19%). System CPU time decreased by 46%.\n\nTwo paired spot checks against GitHub showed end-to-end reductions of 18%\nand 26%. These measurements include network and server variability and are\ntherefore corroborating rather than controlled results. A third pair was not\ninterpretable because the baseline request encountered a transport stall.\n\nA full-clone control showed no material change, taking approximately 256\nseconds with either version. This is expected because full clones do not\nexercise promisor-link recording.\n\nt5302-pack-index.sh passed with both SHA-1 and SHA-256, while\nt0410-partial-clone.sh and t5616-partial-clone.sh also passed. New coverage\nchecks malformed commit headers, conflicting link types, and mismatched tag\ntarget types.\n\nSigned-off-by: Arijit Banerjee <arijit@effectiveailabs.com>\n---\n    index-pack: speed up promisor link recording\n    \n    AI assistance: OpenAI Codex was used to identify the bottleneck and\n    assist with the implementation, testing, and benchmark analysis. I\n    reviewed the resulting change and take responsibility for this\n    submission.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2191%2Farijit91%2Findex-pack-promisor-link-recording-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2191/arijit91/index-pack-promisor-link-recording-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2191\n\n builtin/index-pack.c  | 234 ++++++++++++++++++++++++++++++++++++++----\n t/t5302-pack-index.sh |  50 +++++++++\n 2 files changed, 265 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex bc86925ad0..a973146757 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -23,7 +23,8 @@\n #include \"odb.h\"\n #include \"odb/streaming.h\"\n #include \"oid-array.h\"\n-#include \"oidset.h\"\n+#include \"hash-lookup.h\"\n+#include \"oidmap.h\"\n #include \"path.h\"\n #include \"replace-object.h\"\n #include \"tree-walk.h\"\n@@ -105,6 +106,12 @@ static size_t base_cache_limit;\n struct thread_local_data {\n \tpthread_t thread;\n \tint pack_fd;\n+\tstruct oidmap outgoing_links;\n+};\n+\n+struct outgoing_link {\n+\tstruct oidmap_entry entry;\n+\tenum object_type type;\n };\n \n /* Remember to update object flag allocation in object.h */\n@@ -155,11 +162,8 @@ static uint32_t input_crc32;\n static int input_fd, output_fd;\n static const char *curr_pack;\n \n-/*\n- * outgoing_links is guarded by read_mutex, and record_outgoing_links is\n- * read-only in a thread.\n- */\n-static struct oidset outgoing_links = OIDSET_INIT;\n+/* Worker-local maps are merged after all workers have exited. */\n+static struct oidmap outgoing_links = OIDMAP_INIT;\n static int record_outgoing_links;\n \n static struct thread_local_data *thread_data;\n@@ -196,6 +200,55 @@ static inline void unlock_mutex(pthread_mutex_t *mutex)\n \t\tpthread_mutex_unlock(mutex);\n }\n \n+static void record_outgoing_link_to(struct oidmap *map,\n+\t\t\t\t    const struct object_id *oid,\n+\t\t\t\t    enum object_type type)\n+{\n+\tstruct outgoing_link *link = oidmap_get(map, oid);\n+\n+\tif (link) {\n+\t\tif (type != OBJ_ANY && link->type != OBJ_ANY &&\n+\t\t    type != link->type)\n+\t\t\tdie(_(\"object %s is referred to as both a %s and a %s\"),\n+\t\t\t    oid_to_hex(oid), type_name(link->type),\n+\t\t\t    type_name(type));\n+\t\tif (link->type == OBJ_ANY)\n+\t\t\tlink->type = type;\n+\t\treturn;\n+\t}\n+\n+\tCALLOC_ARRAY(link, 1);\n+\toidcpy(&link->entry.oid, oid);\n+\tlink->type = type;\n+\tif (oidmap_put(map, link))\n+\t\tBUG(\"duplicate outgoing link\");\n+}\n+\n+static void merge_outgoing_links(struct oidmap *dest, struct oidmap *src)\n+{\n+\tstruct oidmap_iter iter;\n+\tstruct outgoing_link *link;\n+\n+\twhile ((link = oidmap_iter_first(src, &iter))) {\n+\t\tstruct outgoing_link *old = oidmap_get(dest, &link->entry.oid);\n+\n+\t\toidmap_remove(src, &link->entry.oid);\n+\t\tif (old) {\n+\t\t\tif (link->type != OBJ_ANY && old->type != OBJ_ANY &&\n+\t\t\t    link->type != old->type)\n+\t\t\t\tdie(_(\"object %s is referred to as both a %s and a %s\"),\n+\t\t\t\t    oid_to_hex(&link->entry.oid),\n+\t\t\t\t    type_name(old->type), type_name(link->type));\n+\t\t\tif (old->type == OBJ_ANY)\n+\t\t\t\told->type = link->type;\n+\t\t\tfree(link);\n+\t\t} else if (oidmap_put(dest, link)) {\n+\t\t\tBUG(\"duplicate outgoing link\");\n+\t\t}\n+\t}\n+\toidmap_clear(src, 0);\n+}\n+\n /*\n  * Mutex and conditional variable can't be statically-initialized on Windows.\n  */\n@@ -211,6 +264,7 @@ static void init_thread(void)\n \tCALLOC_ARRAY(thread_data, nr_threads);\n \tfor (i = 0; i < nr_threads; i++) {\n \t\tthread_data[i].pack_fd = xopen(curr_pack, O_RDONLY);\n+\t\toidmap_init(&thread_data[i].outgoing_links, 0);\n \t}\n \n \tthreads_active = 1;\n@@ -221,14 +275,17 @@ static void cleanup_thread(void)\n \tint i;\n \tif (!threads_active)\n \t\treturn;\n-\tthreads_active = 0;\n \tpthread_mutex_destroy(&read_mutex);\n \tpthread_mutex_destroy(&counter_mutex);\n \tpthread_mutex_destroy(&work_mutex);\n \tif (show_stat)\n \t\tpthread_mutex_destroy(&deepest_delta_mutex);\n-\tfor (i = 0; i < nr_threads; i++)\n+\tfor (i = 0; i < nr_threads; i++) {\n+\t\tmerge_outgoing_links(&outgoing_links,\n+\t\t\t\t     &thread_data[i].outgoing_links);\n \t\tclose(thread_data[i].pack_fd);\n+\t}\n+\tthreads_active = 0;\n \tpthread_key_delete(key);\n \tfree(thread_data);\n }\n@@ -818,9 +875,14 @@ static int check_collison(struct object_entry *entry)\n \treturn 0;\n }\n \n-static void record_outgoing_link(const struct object_id *oid)\n+static void record_outgoing_link(const struct object_id *oid,\n+\t\t\t\t enum object_type type)\n {\n-\toidset_insert(&outgoing_links, oid);\n+\tstruct oidmap *map = &outgoing_links;\n+\n+\tif (threads_active && !strict && !do_fsck_object)\n+\t\tmap = &get_thread_data()->outgoing_links;\n+\trecord_outgoing_link_to(map, oid, type);\n }\n \n static void maybe_record_name_entry(const struct name_entry *entry)\n@@ -849,7 +911,97 @@ static void maybe_record_name_entry(const struct name_entry *entry)\n \t * pack, so it won't be GC-ed, the tradeoff seems worth it.\n \t*/\n \tif (S_ISDIR(entry->mode))\n-\t\trecord_outgoing_link(&entry->oid);\n+\t\trecord_outgoing_link(&entry->oid, OBJ_ANY);\n+}\n+\n+static int parse_outgoing_link_oid(const char **buf, const char *tail,\n+\t\t\t\t   const char *header, struct object_id *oid)\n+{\n+\tconst char *end;\n+\tsize_t header_len = strlen(header);\n+\n+\tif (tail - *buf <= header_len + the_hash_algo->hexsz ||\n+\t    memcmp(*buf, header, header_len) ||\n+\t    parse_oid_hex_algop(*buf + header_len, oid, &end,\n+\t\t\t\tthe_hash_algo) ||\n+\t    end >= tail || *end != '\\n')\n+\t\treturn -1;\n+\t*buf = end + 1;\n+\treturn 0;\n+}\n+\n+static void record_outgoing_links_from_data(const void *data,\n+\t\t\t\t\t    unsigned long size,\n+\t\t\t\t\t    enum object_type type,\n+\t\t\t\t\t    const struct object_id *oid)\n+{\n+\tconst char *buf = data;\n+\tconst char *tail = buf + size;\n+\n+\tif (type == OBJ_TREE) {\n+\t\tstruct tree_desc desc;\n+\t\tstruct name_entry entry;\n+\n+\t\tif (init_tree_desc_gently(&desc, oid, data, size, 0))\n+\t\t\treturn;\n+\t\twhile (tree_entry_gently(&desc, &entry))\n+\t\t\tmaybe_record_name_entry(&entry);\n+\t} else if (type == OBJ_COMMIT) {\n+\t\tstruct object_id link;\n+\t\tstruct commit_graft *graft;\n+\t\tint i;\n+\n+\t\tif (threads_active &&\n+\t\t    !the_repository->parsed_objects->commit_graft_prepared)\n+\t\t\tBUG(\"commit grafts were not prepared before resolving deltas\");\n+\t\tgraft = lookup_commit_graft(the_repository, oid);\n+\n+\t\tif (parse_outgoing_link_oid(&buf, tail, \"tree \", &link))\n+\t\t\tdie(_(\"invalid tree line in commit %s\"),\n+\t\t\t    oid_to_hex(oid));\n+\t\tif (buf >= tail)\n+\t\t\tdie(_(\"truncated commit %s after tree line\"),\n+\t\t\t    oid_to_hex(oid));\n+\t\trecord_outgoing_link(&link, OBJ_TREE);\n+\n+\t\twhile (tail - buf > 7 + the_hash_algo->hexsz &&\n+\t\t       starts_with(buf, \"parent \")) {\n+\t\t\tif (parse_outgoing_link_oid(&buf, tail, \"parent \",\n+\t\t\t\t\t\t    &link))\n+\t\t\t\tdie(_(\"invalid parent line in commit %s\"),\n+\t\t\t\t    oid_to_hex(oid));\n+\t\t\tif (buf >= tail)\n+\t\t\t\tdie(_(\"truncated commit %s after parent line\"),\n+\t\t\t\t    oid_to_hex(oid));\n+\t\t\tif (!graft ||\n+\t\t\t    (graft->nr_parent >= 0 && grafts_keep_true_parents))\n+\t\t\t\trecord_outgoing_link(&link, OBJ_COMMIT);\n+\t\t}\n+\t\tif (graft)\n+\t\t\tfor (i = 0; i < graft->nr_parent; i++)\n+\t\t\t\trecord_outgoing_link(&graft->parent[i],\n+\t\t\t\t\t\t     OBJ_COMMIT);\n+\t} else if (type == OBJ_TAG) {\n+\t\tstruct object_id link;\n+\t\tconst char *line_end;\n+\t\tenum object_type target_type;\n+\n+\t\tif (size < the_hash_algo->hexsz + 24 ||\n+\t\t    parse_outgoing_link_oid(&buf, tail, \"object \", &link))\n+\t\t\tdie(_(\"invalid object line in tag %s\"), oid_to_hex(oid));\n+\t\tif (!skip_prefix(buf, \"type \", &buf) ||\n+\t\t    !(line_end = memchr(buf, '\\n', tail - buf)))\n+\t\t\tdie(_(\"invalid type line in tag %s\"), oid_to_hex(oid));\n+\t\ttarget_type = type_from_string_gently(buf, line_end - buf, 1);\n+\t\tif (target_type < 0)\n+\t\t\tdie(_(\"invalid type line in tag %s\"), oid_to_hex(oid));\n+\t\tbuf = line_end + 1;\n+\t\tif (buf + 4 >= tail || !skip_prefix(buf, \"tag \", &buf) ||\n+\t\t    !memchr(buf, '\\n', tail - buf))\n+\t\t\tdie(_(\"invalid tag name line in tag %s\"),\n+\t\t\t    oid_to_hex(oid));\n+\t\trecord_outgoing_link(&link, target_type);\n+\t}\n }\n \n static void do_record_outgoing_links(struct object *obj)\n@@ -871,12 +1023,13 @@ static void do_record_outgoing_links(struct object *obj)\n \t\tstruct commit *commit = (struct commit *) obj;\n \t\tstruct commit_list *parents = commit->parents;\n \n-\t\trecord_outgoing_link(get_commit_tree_oid(commit));\n+\t\trecord_outgoing_link(get_commit_tree_oid(commit), OBJ_TREE);\n \t\tfor (; parents; parents = parents->next)\n-\t\t\trecord_outgoing_link(&parents->item->object.oid);\n+\t\t\trecord_outgoing_link(&parents->item->object.oid,\n+\t\t\t\t\t     OBJ_COMMIT);\n \t} else if (obj->type == OBJ_TAG) {\n \t\tstruct tag *tag = (struct tag *) obj;\n-\t\trecord_outgoing_link(get_tagged_oid(tag));\n+\t\trecord_outgoing_link(get_tagged_oid(tag), tag->tagged->type);\n \t}\n }\n \n@@ -925,6 +1078,12 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,\n \t\tfree(has_data);\n \t}\n \n+\tif (record_outgoing_links && !strict && !do_fsck_object) {\n+\t\tif (type != OBJ_BLOB)\n+\t\t\trecord_outgoing_links_from_data(data, size, type, oid);\n+\t\tgoto out;\n+\t}\n+\n \tif (strict || do_fsck_object || record_outgoing_links) {\n \t\tread_lock();\n \t\tif (type == OBJ_BLOB) {\n@@ -975,6 +1134,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,\n \t\tread_unlock();\n \t}\n \n+out:\n \tfree(new_data);\n }\n \n@@ -1811,20 +1971,53 @@ static void show_pack_info(int stat_only)\n \tfree(chain_histogram);\n }\n \n+static const struct object_id *idx_object_oid(size_t pos, const void *table)\n+{\n+\tstruct pack_idx_entry * const *entries = table;\n+\n+\treturn &entries[pos]->oid;\n+}\n+\n+static void validate_outgoing_link_types(struct pack_idx_entry **sorted,\n+\t\t\t\t\t int nr)\n+{\n+\tstruct oidmap_iter iter;\n+\tstruct outgoing_link *link;\n+\n+\toidmap_iter_init(&outgoing_links, &iter);\n+\twhile ((link = oidmap_iter_next(&iter))) {\n+\t\tint pos;\n+\t\tstruct object_entry *actual;\n+\n+\t\tif (link->type == OBJ_ANY)\n+\t\t\tcontinue;\n+\t\tpos = oid_pos(&link->entry.oid, sorted, nr, idx_object_oid);\n+\t\tif (pos < 0)\n+\t\t\tcontinue;\n+\t\tactual = container_of(sorted[pos], struct object_entry, idx);\n+\t\tif (actual->real_type != link->type)\n+\t\t\tdie(_(\"object %s is a %s, but was referred to as a %s\"),\n+\t\t\t    oid_to_hex(&link->entry.oid),\n+\t\t\t    type_name(actual->real_type),\n+\t\t\t    type_name(link->type));\n+\t}\n+}\n+\n static void repack_local_links(void)\n {\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n \tFILE *out;\n \tstruct strbuf line = STRBUF_INIT;\n-\tstruct oidset_iter iter;\n-\tstruct object_id *oid;\n+\tstruct oidmap_iter iter;\n+\tstruct outgoing_link *link;\n \tchar *base_name = NULL;\n \n-\tif (!oidset_size(&outgoing_links))\n+\tif (!oidmap_get_size(&outgoing_links))\n \t\treturn;\n \n-\toidset_iter_init(&outgoing_links, &iter);\n-\twhile ((oid = oidset_iter_next(&iter))) {\n+\toidmap_iter_init(&outgoing_links, &iter);\n+\twhile ((link = oidmap_iter_next(&iter))) {\n+\t\tconst struct object_id *oid = &link->entry.oid;\n \t\tstruct odb_source_info source_info;\n \t\tstruct object_info info = {\n \t\t\t.source_infop = &source_info,\n@@ -1919,6 +2112,7 @@ int cmd_index_pack(int argc,\n \tfsck_options.walk = mark_link;\n \n \treset_pack_idx_option(&opts);\n+\toidmap_init(&outgoing_links, 0);\n \topts.flags |= WRITE_REV;\n \trepo_config(the_repository, git_index_pack_config, &opts);\n \tif (prefix && chdir(prefix))\n@@ -2102,6 +2296,7 @@ int cmd_index_pack(int argc,\n \t\tidx_objects[i] = &objects[i].idx;\n \tcurr_index = write_idx_file(the_repository, index_name, idx_objects,\n \t\t\t\t    nr_objects, &opts, pack_hash);\n+\tvalidate_outgoing_link_types(idx_objects, nr_objects);\n \tif (rev_index)\n \t\tcurr_rev_index = write_rev_file(the_repository, rev_index_name,\n \t\t\t\t\t\tidx_objects, nr_objects,\n@@ -2146,6 +2341,7 @@ int cmd_index_pack(int argc,\n \tfree(curr_rev_index);\n \n \trepack_local_links();\n+\toidmap_clear(&outgoing_links, 1);\n \n \t/*\n \t * Let the caller know this pack is not self contained\ndiff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh\nindex 735de1023e..2f99a49d9d 100755\n--- a/t/t5302-pack-index.sh\n+++ b/t/t5302-pack-index.sh\n@@ -309,4 +309,54 @@ test_expect_success DEFAULT_HASH_ALGORITHM 'index-pack --fsck-objects outside of\n \t)\n '\n \n+test_expect_success 'index-pack --promisor rejects malformed commits' '\n+\ttest_when_finished \"rm -rf malformed-src malformed-dst malformed.pack bad-commit\" &&\n+\ttest_create_repo malformed-src &&\n+\ttest_create_repo malformed-dst &&\n+\tprintf \"tree not-an-object\\n\\nmessage\\n\" >bad-commit &&\n+\tbad_oid=$(git -C malformed-src hash-object --literally -t commit \\\n+\t\t-w --stdin <bad-commit) &&\n+\tprintf \"%s\\n\" \"$bad_oid\" |\n+\t\tgit -C malformed-src pack-objects --stdout >malformed.pack &&\n+\ttest_must_fail git -C malformed-dst index-pack --stdin --promisor \\\n+\t\t<malformed.pack 2>err &&\n+\ttest_grep \"invalid tree line in commit $bad_oid\" err\n+'\n+\n+test_expect_success 'index-pack --promisor rejects conflicting link types' '\n+\ttest_when_finished \"rm -rf conflict-src conflict-dst conflict.pack bad-commit\" &&\n+\ttest_create_repo conflict-src &&\n+\ttest_create_repo conflict-dst &&\n+\ttree_oid=$(git -C conflict-src mktree </dev/null) &&\n+\t{\n+\t\tprintf \"tree %s\\n\" \"$tree_oid\" &&\n+\t\tprintf \"parent %s\\n\\nmessage\\n\" \"$tree_oid\"\n+\t} >bad-commit &&\n+\tcommit_oid=$(git -C conflict-src hash-object --literally -t commit \\\n+\t\t-w --stdin <bad-commit) &&\n+\tprintf \"%s\\n%s\\n\" \"$tree_oid\" \"$commit_oid\" |\n+\t\tgit -C conflict-src pack-objects --stdout >conflict.pack &&\n+\ttest_must_fail git -C conflict-dst index-pack --stdin --promisor \\\n+\t\t<conflict.pack 2>err &&\n+\ttest_grep \"object $tree_oid is referred to as both a tree and a commit\" err\n+'\n+\n+test_expect_success 'index-pack --promisor verifies tag target types' '\n+\ttest_when_finished \"rm -rf tag-src tag-dst tag.pack bad-tag\" &&\n+\ttest_create_repo tag-src &&\n+\ttest_create_repo tag-dst &&\n+\ttree_oid=$(git -C tag-src mktree </dev/null) &&\n+\t{\n+\t\tprintf \"object %s\\n\" \"$tree_oid\" &&\n+\t\tprintf \"type commit\\ntag wrong-type\\n\\nmessage\\n\"\n+\t} >bad-tag &&\n+\ttag_oid=$(git -C tag-src hash-object --literally -t tag \\\n+\t\t-w --stdin <bad-tag) &&\n+\tprintf \"%s\\n%s\\n\" \"$tree_oid\" \"$tag_oid\" |\n+\t\tgit -C tag-src pack-objects --stdout >tag.pack &&\n+\ttest_must_fail git -C tag-dst index-pack --stdin --promisor \\\n+\t\t<tag.pack 2>err &&\n+\ttest_grep \"object $tree_oid is a tree, but was referred to as a commit\" err\n+'\n+\n test_done\n\nbase-commit: a97fcc37c2bc6340a8d7ce78dedf227aac4e9aa7\n-- \ngitgitgadget\n"},{"id":"549450","messageId":"am-7_wSb-GNefKlB@fruit.crustytoothpaste.net","threadId":"66103","inReplyTo":"pull.2191.git.1785706396130.gitgitgadget@gmail.com","subject":"Re: [PATCH] index-pack: speed up promisor link recording","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-08-02T21:51:59Z","receivedAt":"2026-08-02T21:52:07Z","isPatch":true,"body":"On 2026-08-02 at 21:33:15, Arijit Banerjee via GitGitGadget wrote:\n> From: Arijit Banerjee <arijit@effectiveailabs.com>\n> \n> When indexing a promisor pack, index-pack parses every reconstructed\n> non-blob object into the shared object model to record its outgoing links.\n> Since parse_object_buffer() runs under read_mutex, worker threads serialize\n> while allocating persistent tree, commit, and tag structures that are only\n> needed to enumerate those links.\n> \n> Read the links directly from the reconstructed object buffers instead. Keep\n> the strict and fsck paths unchanged, use worker-local typed oidmaps during\n> normal promisor indexing, and merge them after the workers exit. Transfer\n> entries during the merge so that it does not temporarily duplicate the\n> complete link set.\n> \n> The typed entries preserve checks previously performed as a side effect of\n> object parsing. Reject malformed commit and tag headers, conflicting\n> expected types, and targets whose actual type disagrees when the target is\n> present in the pack. Preserve commit-graft handling and the existing policy\n> of recording only subtree entries from trees.\n> \n> With three runs per version on Debian 12, median end-to-end wall-clock time\n> for a --filter=blob:none clone of linux.git decreased from 156 seconds to\n> 133 seconds (15%). Trace2 attributed the change to the initial index-pack\n> --promisor phase, whose median duration decreased from 121 seconds to 98\n> seconds (19%). System CPU time decreased by 46%.\n> \n> Two paired spot checks against GitHub showed end-to-end reductions of 18%\n> and 26%. These measurements include network and server variability and are\n> therefore corroborating rather than controlled results. A third pair was not\n> interpretable because the baseline request encountered a transport stall.\n> \n> A full-clone control showed no material change, taking approximately 256\n> seconds with either version. This is expected because full clones do not\n> exercise promisor-link recording.\n> \n> t5302-pack-index.sh passed with both SHA-1 and SHA-256, while\n> t0410-partial-clone.sh and t5616-partial-clone.sh also passed. New coverage\n> checks malformed commit headers, conflicting link types, and mismatched tag\n> target types.\n> \n> Signed-off-by: Arijit Banerjee <arijit@effectiveailabs.com>\n> ---\n>     index-pack: speed up promisor link recording\n> \n>     AI assistance: OpenAI Codex was used to identify the bottleneck and\n>     assist with the implementation, testing, and benchmark analysis. I\n>     reviewed the resulting change and take responsibility for this\n>     submission.\n\nI don't think SubmittingPatches really allows more than trivial changes\nwritten by AI:\n\n    The Developer's Certificate of Origin requires contributors to certify\n    that they know the origin of their contributions to the project and\n    that they have the right to submit it under the project's license.\n    It's not yet clear that this can be legally satisfied when submitting\n    significant amount of content that has been generated by AI tools.\n\n    [...]\n\n    To avoid these issues, we will reject anything that looks AI\n    generated, that sounds overly formal or bloated, that looks like AI\n    slop, that looks good on the surface but makes no sense, or that\n    senders don’t understand or cannot explain.\n\nThis doesn't look like it's a trivial change, so I don't believe this\npatch can be accepted.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"549453","messageId":"CAFwoC-5R7VLHzXQ1WY5fMe6Od--VcP0FzR-AQHk2OEt6WVLSEg@mail.gmail.com","threadId":"66103","inReplyTo":"am-7_wSb-GNefKlB@fruit.crustytoothpaste.net","subject":"Re: [PATCH] index-pack: speed up promisor link recording","fromName":"Arijit Banerjee","fromEmail":"arijit@effectiveailabs.com","sentAt":"2026-08-02T22:20:10Z","receivedAt":"2026-08-02T22:20:23Z","isPatch":true,"body":"On Sun, Aug 2, 2026, brian m. carlson wrote:\n> This doesn't look like it's a trivial change, so I don't believe this\n> patch can be accepted.\n\nThanks, Brian. I am not trying to bypass the project's policy.\n\nI do not claim to be an expert on this topic, but Codex appears to have\nfound a material performance improvement of about 15% on end-to-end\nblobless clone times. Would it be appropriate to treat the current\nsubmission as an RFC for maintainers before deciding if the optimization is\nworth getting in? It seems worth trying to preserve the technical\nresult.\n\nThanks,\nArijit\n\n\nOn Sun, Aug 2, 2026 at 2:52 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2026-08-02 at 21:33:15, Arijit Banerjee via GitGitGadget wrote:\n> > From: Arijit Banerjee <arijit@effectiveailabs.com>\n> >\n> > When indexing a promisor pack, index-pack parses every reconstructed\n> > non-blob object into the shared object model to record its outgoing links.\n> > Since parse_object_buffer() runs under read_mutex, worker threads serialize\n> > while allocating persistent tree, commit, and tag structures that are only\n> > needed to enumerate those links.\n> >\n> > Read the links directly from the reconstructed object buffers instead. Keep\n> > the strict and fsck paths unchanged, use worker-local typed oidmaps during\n> > normal promisor indexing, and merge them after the workers exit. Transfer\n> > entries during the merge so that it does not temporarily duplicate the\n> > complete link set.\n> >\n> > The typed entries preserve checks previously performed as a side effect of\n> > object parsing. Reject malformed commit and tag headers, conflicting\n> > expected types, and targets whose actual type disagrees when the target is\n> > present in the pack. Preserve commit-graft handling and the existing policy\n> > of recording only subtree entries from trees.\n> >\n> > With three runs per version on Debian 12, median end-to-end wall-clock time\n> > for a --filter=blob:none clone of linux.git decreased from 156 seconds to\n> > 133 seconds (15%). Trace2 attributed the change to the initial index-pack\n> > --promisor phase, whose median duration decreased from 121 seconds to 98\n> > seconds (19%). System CPU time decreased by 46%.\n> >\n> > Two paired spot checks against GitHub showed end-to-end reductions of 18%\n> > and 26%. These measurements include network and server variability and are\n> > therefore corroborating rather than controlled results. A third pair was not\n> > interpretable because the baseline request encountered a transport stall.\n> >\n> > A full-clone control showed no material change, taking approximately 256\n> > seconds with either version. This is expected because full clones do not\n> > exercise promisor-link recording.\n> >\n> > t5302-pack-index.sh passed with both SHA-1 and SHA-256, while\n> > t0410-partial-clone.sh and t5616-partial-clone.sh also passed. New coverage\n> > checks malformed commit headers, conflicting link types, and mismatched tag\n> > target types.\n> >\n> > Signed-off-by: Arijit Banerjee <arijit@effectiveailabs.com>\n> > ---\n> >     index-pack: speed up promisor link recording\n> >\n> >     AI assistance: OpenAI Codex was used to identify the bottleneck and\n> >     assist with the implementation, testing, and benchmark analysis. I\n> >     reviewed the resulting change and take responsibility for this\n> >     submission.\n>\n> I don't think SubmittingPatches really allows more than trivial changes\n> written by AI:\n>\n>     The Developer's Certificate of Origin requires contributors to certify\n>     that they know the origin of their contributions to the project and\n>     that they have the right to submit it under the project's license.\n>     It's not yet clear that this can be legally satisfied when submitting\n>     significant amount of content that has been generated by AI tools.\n>\n>     [...]\n>\n>     To avoid these issues, we will reject anything that looks AI\n>     generated, that sounds overly formal or bloated, that looks like AI\n>     slop, that looks good on the surface but makes no sense, or that\n>     senders don’t understand or cannot explain.\n>\n> This doesn't look like it's a trivial change, so I don't believe this\n> patch can be accepted.\n> --\n> brian m. carlson (they/them)\n> Toronto, Ontario, CA\n"},{"id":"549455","messageId":"am_Fb79hCnwmRzjL@fruit.crustytoothpaste.net","threadId":"66103","inReplyTo":"CAFwoC-7wUzce_XvuviXZe=5eTxJ5yyCpz=vsOheWKPCnz9Kr4A@mail.gmail.com","subject":"Re: [PATCH] index-pack: speed up promisor link recording","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-08-02T22:32:16Z","receivedAt":"2026-08-02T22:32:18Z","isPatch":true,"body":"On 2026-08-02 at 22:12:16, Arijit Banerjee wrote:\n> Thanks, Brian. I am not trying to bypass the project's policy.\n> \n> The investigation is in the same general spirit as the Git performance work\n> being tracked here:\n> https://openai-git-upstream.openai.chatgpt.site/\n> \n> I do not claim to be an expert on this topic, but Codex appears to have found\n> a material performance improvement of about 15% on end-to-end blobless clone\n> times.\n> \n> Would it be appropriate to treat the current submission as an RFC? It seems\n> worth trying to preserve the technical result.\n\nI don't think the project's policy prevents you from doing analysis and\ninvestigation with an LLM, although it does require you to verify the\ncorrectness of the results and be accountable for them.  If, based on\nthe analysis of the performance impact, you write some code without the\nuse of an LLM that improves things, I think that would be allowed and\nprobably welcome, assuming it is otherwise acceptable.  Some\ncontributors will be willing to review such a contribution and others\nwill not, but it is not outside of the policy.\n\nHowever, writing substantial code with an LLM doesn't appear to be\nallowed.  The kinds of trivial changes that I think would be allowed to\nbe generated would be things like fixing spelling errors or adding\ninclude guards to header files that lack them.  Of course, these are\nalso the kinds of things you could mostly fix with a small script, which\nis why they are generally considered so trivial as to be\nuncopyrightable.\n\nSo I think to have a patch accepted in this case, you would need to\ntotally discard the existing patch and rewrite it by hand without\nrecourse to the generated code.\n\nI understand that the SubmittingPatches documentation is a bit long, but\nI do suggest giving it at least a glance so you know what to expect.  I\nthink reading this sort of contributing documentation is more important\nthan ever since, in the era of LLMs, projects tend to have strong\nopinions on what is and is not acceptable, not only just in terms of LLM\nusage, but in how code and documentation are to be written and\nformatted.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"549458","messageId":"xmqqcxw02lao.fsf@gitster.g","threadId":"66103","inReplyTo":"am-7_wSb-GNefKlB@fruit.crustytoothpaste.net","subject":"Re: [PATCH] index-pack: speed up promisor link recording","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-02T22:46:07Z","receivedAt":"2026-08-02T22:46:10Z","isPatch":true,"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n>>     index-pack: speed up promisor link recording\n>> \n>>     AI assistance: OpenAI Codex was used to identify the bottleneck and\n>>     assist with the implementation, testing, and benchmark analysis. I\n>>     reviewed the resulting change and take responsibility for this\n>>     submission.\n>\n> I don't think SubmittingPatches really allows more than trivial changes\n> written by AI:\n>\n>     The Developer's Certificate of Origin requires contributors to certify\n>     that they know the origin of their contributions to the project and\n>     that they have the right to submit it under the project's license.\n>     It's not yet clear that this can be legally satisfied when submitting\n>     significant amount of content that has been generated by AI tools.\n>\n>     [...]\n>\n>     To avoid these issues, we will reject anything that looks AI\n>     generated, that sounds overly formal or bloated, that looks like AI\n>     slop, that looks good on the surface but makes no sense, or that\n>     senders don’t understand or cannot explain.\n>\n> This doesn't look like it's a trivial change, so I don't believe this\n> patch can be accepted.\n\nThe project we borrowed DCO from has this to say on this topic:\n\n https://docs.kernel.org/process/coding-assistants.html\n\n * All contributions must comply with licensing reuqirements.\n * Only humans can attest DCO by Siging off their patches.\n\n   The human submitter is responsible for reviewing all AI generated\n   code, ensuring compliance with licensing requirements, certify\n   DCO with their sign-off, and taking full responsibility for the\n   contribution.\n\nNow we are *not* kernel, but I think there is a general concensus in\nthe community that, while we do not want to outright ban machine\nassisted contributions, we generally want to tread very carefully,\nespecially in the DCO area.\n\nIt is very easy for anybody and their dog to say \"I reviewed X\" and\nit is very hard for others to assess how trustworthy such a\nstatement is, so I am unsure how the kernel project is enforcing the\n\"human submitter is responsible for these\", and more importantly,\neven assuming that we would take a similar policy for ourselves, I\nam not sure what mechanism we can put in place to detect cases where\nthese expectations are violated.\n\nSo...\n"},{"id":"549460","messageId":"xmqq7bm82l05.fsf@gitster.g","threadId":"66103","inReplyTo":"am_Fb79hCnwmRzjL@fruit.crustytoothpaste.net","subject":"Re: [PATCH] index-pack: speed up promisor link recording","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-02T22:52:26Z","receivedAt":"2026-08-02T23:01:16Z","isPatch":true,"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> I understand that the SubmittingPatches documentation is a bit long, but\n> I do suggest giving it at least a glance so you know what to expect.  I\n> think reading this sort of contributing documentation is more important\n> than ever since, in the era of LLMs, projects tend to have strong\n> opinions on what is and is not acceptable, not only just in terms of LLM\n> usage, but in how code and documentation are to be written and\n> formatted.\n\nAmen.\n\nSince we seem to be drawn into the AI policy discussion, are there\nthings that we should consider borrowing from policies battle-tested\nby other projects?  I kind of like what LLVM has as \"extractive\ncontributions are rejected (whether it is AI or not AI)\", as we are\nseverely review-bandwidth limited these days.\n\n"},{"id":"549461","messageId":"CAFwoC-6eJNX4H7AeOcTdSbFRqkoqAOb2Js9Qj+93kHJWG7OkUQ@mail.gmail.com","threadId":"66103","inReplyTo":"xmqq7bm82l05.fsf@gitster.g","subject":"Re: [PATCH] index-pack: speed up promisor link recording","fromName":"Arijit Banerjee","fromEmail":"arijit@effectiveailabs.com","sentAt":"2026-08-02T23:19:41Z","receivedAt":"2026-08-02T23:19:52Z","isPatch":true,"body":"Two suggestions:\n- Maybe a karma system?\n- A special release train that is more indulgent towards AI generated\ncode? Brave users can try out features and they get baked into stable\nreleases after enough soak time\n\nDefinitely not trying to be extractive, this one appears to be a\ndecent size perf improvement!\n\nStill holding on to a SHA1-DC hardware acceleration change, that one\nwould admittedly be much harder to review ;)\n\n\nOn Sun, Aug 2, 2026 at 3:52 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n>\n> > I understand that the SubmittingPatches documentation is a bit long, but\n> > I do suggest giving it at least a glance so you know what to expect.  I\n> > think reading this sort of contributing documentation is more important\n> > than ever since, in the era of LLMs, projects tend to have strong\n> > opinions on what is and is not acceptable, not only just in terms of LLM\n> > usage, but in how code and documentation are to be written and\n> > formatted.\n>\n> Amen.\n>\n> Since we seem to be drawn into the AI policy discussion, are there\n> things that we should consider borrowing from policies battle-tested\n> by other projects?  I kind of like what LLVM has as \"extractive\n> contributions are rejected (whether it is AI or not AI)\", as we are\n> severely review-bandwidth limited these days.\n>\n"},{"id":"549463","messageId":"am_hWvag32v8yuNM@fruit.crustytoothpaste.net","threadId":"66103","inReplyTo":"CAFwoC-6EvoD-u7oceETi90MJ-FQA2zihdkn1i1wckKfoYRTKOw@mail.gmail.com","subject":"Re: [PATCH] index-pack: speed up promisor link recording","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2026-08-03T00:31:23Z","receivedAt":"2026-08-03T00:31:25Z","isPatch":true,"body":"[please avoid top-posting]\n\nOn 2026-08-02 at 22:54:27, Arijit Banerjee wrote:\n> Maybe software can ship experimental versions which is more indulging\n> towards AI generated patches? Brave users get to try the features and they\n> can baked into stable releases once there's enough soak time.\n> \n> I have been holding onto my patch around hardware acceleration for SHA1-DC\n> :)\n\nThe rationale, as Junio said, is based on the Developer's Certificate of\nOrigin.  That is a legal statement that a person has the legal right to\ncontribute those changes under the license and if they make a false or\nmisleading statement to that effect, they are responsible—legally and\notherwise—for it.\n\nConsidering the extensive litigation over LLM output at the moment, I\ndon't think anyone can clearly make that assertion.  Most of the\narguments I've heard are that it's fair use, which is a U.S. legal\nconcept.  That does not exist in Canada or the U.K., where there is fair\ndealing, which is much more restricted.\n\nIf Company X includes LLM-generated code in their proprietary product\nand it's found to be infringing in say, Germany, then they can simply\nnot distribute their code in Germany.  Git cannot do that: it's\ndistributed in Linux distributions around the planet, even in countries\nsubject to sanctions, such as Russia[0].  We must comply with the\nlicense and the law everywhere in every country or we risk liability for\nour contributors and distributors.  I, for one, am not willing to be\nsued over this project and the project does not have the financial means\nto deal with extensive litigation.\n\nThere are also concerns about the quality of the code and whether\nsubmitters adequately understand the code well enough to have evaluated\nand reviewed it thoroughly.  It's well known that when creating code\nbecomes cheap, the burden shifts to review and review becomes extremely\nimportant.  That has been seen in lots of places, but we are an open\nsource project and we can't force contributors to do review like a\ncompany can.  As Junio says, we already have trouble getting reviews\nthrough and we don't want to make the problem worse.\n\nLLM-generated content also has a negative quality reputation (see the\nreaction to AI content in video games and books for an example) and\nwhile all software has bugs, I appreciate the reputation that Git has\nfor quality and wish to retain that.\n\nThose alone are reason enough for the policy, but there are other\nconcerns about the environmental impact, the impact on electricity and\nhardware prices, the ethics of incorporating open source code without so\nmuch as a credit[1], and a lot more.\n\nSo I don't think it's likely we're going to accept nontrivial\nLLM-generated content in any capacity anytime soon and I don't think\ntrying to argue this or persuade us to accept it is going to be\nproductive or well received.  Of course, anyone can distribute their own\nfork of Git with additional patches if they prefer, but we won't include\nthem.\n\n[0] Debian, which distributes Git, has mirrors in Russia and Belarus:\nhttps://www.debian.org/mirror/list\n[1] For instance, as a member of ACM, I have to follow § 1.5 (Respect\nthe work required to produce new ideas, inventions, creative works, and\ncomputing artifacts) of the Code of Ethics:\nhttps://www.acm.org/code-of-ethics.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"549466","messageId":"87ldaonhu4.fsf@gmail.com","threadId":"66103","inReplyTo":"am_hWvag32v8yuNM@fruit.crustytoothpaste.net","subject":"Re: [PATCH] index-pack: speed up promisor link recording","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2026-08-03T00:55:15Z","receivedAt":"2026-08-03T00:55:18Z","isPatch":true,"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> If Company X includes LLM-generated code in their proprietary product\n> and it's found to be infringing in say, Germany, then they can simply\n> not distribute their code in Germany.  Git cannot do that: it's\n> distributed in Linux distributions around the planet, even in countries\n> subject to sanctions, such as Russia[0].  We must comply with the\n> license and the law everywhere in every country or we risk liability for\n> our contributors and distributors.  I, for one, am not willing to be\n> sued over this project and the project does not have the financial means\n> to deal with extensive litigation.\n\nI generally agree. But some countries have more respected legal systems\nthan others, to put it mildly. I certainly hope that being extradited to\none of the sanctioned countries isn't too large of a concern.\n\nCollin\n"}]}