{"thread":{"id":"66262","subject":"[PATCH 1/2] rerere: extract logic to determine whether entries are stale","startedAt":"2026-09-03T09:05:19Z","lastAt":"2026-09-07T06:15:41Z","messageCount":17,"participants":["Patrick Steinhardt","Thomas Bachem","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"551837","messageId":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-1-9929c45a9788@pks.im","threadId":"66262","inReplyTo":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im","subject":"[PATCH 1/2] rerere: extract logic to determine whether entries are stale","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-03T09:04:57Z","receivedAt":"2026-09-03T09:05:19Z","isPatch":true,"body":"When garbage collecting rerere entries we need to figure out whether any\ngiven entry is stale before pruning it. In a subsequent commit we're\nabout to introduce a second caller that wants to determine staleness,\nbut the logic is not currently reusable.\n\nExtract the logic to compute staleness by introducing two new helper\nfunctions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.\n\nSigned-off-by: Patrick Steinhardt \n---\n rerere.c | 42 +++++++++++++++++++++++++++---------------\n 1 file changed, 27 insertions(+), 15 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 3d3bd0db16..d01af6b71b 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -1173,23 +1173,38 @@ static void unlink_rr_item(struct rerere_id *id)\n \tstrbuf_release(&buf);\n }\n \n-static void prune_one(struct rerere_id *id,\n-\t\t timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)\n+static void rerere_gc_cutoffs(struct repository *r,\n+\t\t\t timestamp_t *cutoff_resolve,\n+\t\t\t timestamp_t *cutoff_noresolve)\n+{\n+\ttimestamp_t now = time(NULL);\n+\n+\tif (repo_config_get_expiry_in_days(r, \"gc.rerereresolved\",\n+\t\t\t\t\t cutoff_resolve, now))\n+\t\t*cutoff_resolve = now - 60 * 86400;\n+\tif (repo_config_get_expiry_in_days(r, \"gc.rerereunresolved\",\n+\t\t\t\t\t cutoff_noresolve, now))\n+\t\t*cutoff_noresolve = now - 15 * 86400;\n+}\n+\n+static bool rerere_id_is_stale(struct rerere_id *id,\n+\t\t\t timestamp_t cutoff_resolve,\n+\t\t\t timestamp_t cutoff_noresolve)\n {\n \ttimestamp_t then;\n \ttimestamp_t cutoff;\n \n \tthen = rerere_last_used_at(id);\n-\tif (then)\n+\tif (then) {\n \t\tcutoff = cutoff_resolve;\n-\telse {\n+\t} else {\n \t\tthen = rerere_created_at(id);\n \t\tif (!then)\n-\t\t\treturn;\n+\t\t\treturn false;\n \t\tcutoff = cutoff_noresolve;\n \t}\n-\tif (then < cutoff)\n-\t\tunlink_rr_item(id);\n+\n+\treturn then < cutoff;\n }\n \n /* Does the basename in \"path\" look plausibly like an rr-cache entry? */\n@@ -1206,18 +1221,14 @@ void rerere_gc(struct repository *r, struct string_list *rr)\n \tDIR *dir;\n \tstruct dirent *e;\n \tint i;\n-\ttimestamp_t now = time(NULL);\n-\ttimestamp_t cutoff_noresolve = now - 15 * 86400;\n-\ttimestamp_t cutoff_resolve = now - 60 * 86400;\n+\ttimestamp_t cutoff_noresolve;\n+\ttimestamp_t cutoff_resolve;\n \tstruct strbuf buf = STRBUF_INIT;\n \n \tif (setup_rerere(r, rr, 0) < 0)\n \t\treturn;\n \n-\trepo_config_get_expiry_in_days(the_repository, \"gc.rerereresolved\",\n-\t\t\t\t &cutoff_resolve, now);\n-\trepo_config_get_expiry_in_days(the_repository, \"gc.rerereunresolved\",\n-\t\t\t\t &cutoff_noresolve, now);\n+\trerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);\n \trepo_config(the_repository, git_default_config, NULL);\n \tdir = opendir(repo_git_path_replace(the_repository, &buf, \"rr-cache\"));\n \tif (!dir)\n@@ -1237,7 +1248,8 @@ void rerere_gc(struct repository *r, struct string_list *rr)\n \t\tfor (id.variant = 0, id.collection = rr_dir;\n \t\t id.variant < id.collection->status_nr;\n \t\t id.variant++) {\n-\t\t\tprune_one(&id, cutoff_resolve, cutoff_noresolve);\n+\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))\n+\t\t\t\tunlink_rr_item(&id);\n \t\t\tif (id.collection->status[id.variant])\n \t\t\t\tnow_empty = 0;\n \t\t}\n\n-- \n2.55.0.979.g7e5102b832.dirty\n\n"},{"id":"551838","messageId":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im","threadId":"66262","inReplyTo":null,"subject":"[PATCH 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-03T09:04:56Z","receivedAt":"2026-09-03T09:05:19Z","isPatch":true,"body":"Hi,\n\nas reported and discussed in [1]. Thanks!\n\nPatrick\n\n[1]: \n\n---\nPatrick Steinhardt (2):\n rerere: extract logic to determine whether entries are stale\n builtin/maintenance: improve heuristic for \"rerere gc\"\n\n Documentation/config/maintenance.adoc | 8 ++--\n builtin/gc.c | 26 ++--------\n rerere.c | 89 +++++++++++++++++++++++++++++------\n rerere.h | 6 +++\n t/t7900-maintenance.sh | 61 ++++++++++++++++++------\n 5 files changed, 135 insertions(+), 55 deletions(-)\n\n\n---\nbase-commit: 3cb9185f65410273787f74333cc027d2ea5daada\nchange-id: 20260903-b4-pks-maintenance-rerere-gc-heuristic-763b0a9a50d2\n\n"},{"id":"551839","messageId":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-2-9929c45a9788@pks.im","threadId":"66262","inReplyTo":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im","subject":"[PATCH 2/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-03T09:04:58Z","receivedAt":"2026-09-03T09:05:20Z","isPatch":true,"body":"The \"rerere-gc\" maintenance task is responsible for pruning rerere\nentries older than a certain configurable cutoff point. Whether or not\nthe task gets run during auto-maintenance can be configured via\n\"maintenance.rerere-gc.auto\":\n\n - A negative value indicates that maintenance should always run.\n\n - A zero value indicates that maintenance should never run.\n\n - Otherwise, a positive value indicates that maintenance should always\n run in case we have at least a single rerere entry.\n\nWhile the first two conditions are sensible, the last one is less so as\nit does not account for whether we would even prune old entries in the\nfirst place. Instead, it effectively implies that we unconditionally\nspawn \"git rerere gc\" when rerere is enabled. Chances are high though\nthat there is nothing to prune, as the default cutoff dates are 60 days\nfor resolved rerere entries and 15 days for unresolved ones.\n\nBesides being a waste of compute, it also obstructs concurrent processes\nthat want to write new resolutions as garbage collection takes a central\nlock file, as reported in [1]. That race is a longstanding one that\nexisted even before we introduced fine-grained maintenance tasks, and\nthe proper fix is to use a locking timeout in the writing processes. But\nthe race is made worse by us performing garbage collection a lot more\noften.\n\nRefine the heuristic to take into account whether any entries can be\npruned in the first place. This ensures that we'll only ever run this\ntask in situations where it will do anything, and should thus result in\na lot less frequent invocations of \"git rerere gc\".\n\nFurthermore, tweak the meaning of \"maintenance.rerere-gc.auto\" so that\npositive values allow the user to configure the number of prunable\nentries that need to exist before we run it and set the default value to\n512. This number is pulled out of thin air, but it ensures that we know\nto batch-delete entries instead of pruning every single entry that is\nolder than the cutoff point.\n\nNote that this now requires us to actually open the rerere-entry\ndirectories and stat the individual files in there, which does add a bit\nof overhead when one has lots of rerere entries. To counteract this\noverhead, we thus use the same sampling heuristic as we do for loose\nobjects, where we only consider those entries that start with a \"17\".\n\n[1]: \n\nReported-by: Thomas Bachem \nSigned-off-by: Patrick Steinhardt \n---\n Documentation/config/maintenance.adoc | 8 ++---\n builtin/gc.c | 26 +++------------\n rerere.c | 47 +++++++++++++++++++++++++++\n rerere.h | 6 ++++\n t/t7900-maintenance.sh | 61 +++++++++++++++++++++++++++--------\n 5 files changed, 108 insertions(+), 40 deletions(-)\n\ndiff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc\nindex da8be9f812..77977dcc48 100644\n--- a/Documentation/config/maintenance.adoc\n+++ b/Documentation/config/maintenance.adoc\n@@ -121,10 +121,10 @@ maintenance.rerere-gc.auto::\n \tThis integer config option controls how often the `rerere-gc` task\n \tshould be run as part of `git maintenance run --auto`. If zero, then\n \tthe `rerere-gc` task will not run with the `--auto` option. A negative\n-\tvalue will force the task to run every time. Otherwise, any positive\n-\tvalue implies the command will run when the \"rr-cache\" directory exists\n-\tand has at least one entry, regardless of whether it is stale or not.\n-\tThis heuristic may be refined in the future. The default value is 1.\n+\tvalue will force the task to run every time. Otherwise, a positive\n+\tvalue implies the command should run when the estimated number of stale\n+\tentries that would be pruned is greater than or equal to the configured\n+\tvalue. The default value is 512.\n \n maintenance.worktree-prune.auto::\n \tThis integer config option controls how often the `worktree-prune` task\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex de2f9e7fed..9147418a61 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -396,31 +396,13 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,\n \n static int rerere_gc_condition(struct gc_config *cfg UNUSED)\n {\n-\tstruct strbuf path = STRBUF_INIT;\n-\tint should_gc = 0, limit = 1;\n-\tDIR *dir = NULL;\n+\tint limit = 512;\n \n \trepo_config_get_int(the_repository, \"maintenance.rerere-gc.auto\", &limit);\n-\tif (limit <= 0) {\n-\t\tshould_gc = limit < 0;\n-\t\tgoto out;\n-\t}\n-\n-\t/*\n-\t * We skip garbage collection in case we either have no \"rr-cache\"\n-\t * directory or when it doesn't contain at least one entry.\n-\t */\n-\trepo_git_path_replace(the_repository, &path, \"rr-cache\");\n-\tdir = opendir(path.buf);\n-\tif (!dir)\n-\t\tgoto out;\n-\tshould_gc = !!readdir_skip_dot_and_dotdot(dir);\n+\tif (limit <= 0)\n+\t\treturn limit < 0;\n \n-out:\n-\tstrbuf_release(&path);\n-\tif (dir)\n-\t\tclosedir(dir);\n-\treturn should_gc;\n+\treturn rerere_gc_estimate(the_repository, limit) >= (size_t)limit;\n }\n \n #define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \\\ndiff --git a/rerere.c b/rerere.c\nindex d01af6b71b..87a42c4cc3 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -1215,6 +1215,53 @@ static int is_rr_cache_dirname(const char *path)\n \treturn !parse_oid_hex(path, &oid, &end) && !*end;\n }\n \n+size_t rerere_gc_estimate(struct repository *r, size_t limit)\n+{\n+\ttimestamp_t cutoff_resolve, cutoff_noresolve;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct dirent *e;\n+\tsize_t count = 0;\n+\tDIR *dir;\n+\n+\tdir = opendir(repo_git_path_replace(r, &buf, \"rr-cache\"));\n+\tif (!dir)\n+\t\tgoto out;\n+\n+\trerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);\n+\n+\twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n+\t\tstruct rerere_id id;\n+\n+\t\t/*\n+\t\t * We estimate the number of stale entries by only considering\n+\t\t * those starting with \"17\". This is the same strategy that we\n+\t\t * use for estimating the number of loose objects.\n+\t\t */\n+\t\tif (!starts_with(e->d_name, \"17\") ||\n+\t\t !is_rr_cache_dirname(e->d_name))\n+\t\t\tcontinue;\n+\n+\t\tid.collection = find_rerere_dir(e->d_name);\n+\t\tfor (id.variant = 0;\n+\t\t id.variant < id.collection->status_nr;\n+\t\t id.variant++) {\n+\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve,\n+\t\t\t\t\t cutoff_noresolve)) {\n+\t\t\t\tcount += 256;\n+\t\t\t\tif (count >= limit)\n+\t\t\t\t\tgoto out;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+out:\n+\tif (dir)\n+\t\tclosedir(dir);\n+\tfree_rerere_dirs();\n+\tstrbuf_release(&buf);\n+\treturn count;\n+}\n+\n void rerere_gc(struct repository *r, struct string_list *rr)\n {\n \tstruct string_list to_remove = STRING_LIST_INIT_DUP;\ndiff --git a/rerere.h b/rerere.h\nindex d4b5f7c932..898ebdd25a 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -39,6 +39,12 @@ int rerere_remaining(struct repository *, struct string_list *);\n void rerere_clear(struct repository *, struct string_list *);\n void rerere_gc(struct repository *, struct string_list *);\n \n+/*\n+ * Estimate the number of stale entries that a run of \"git rerere gc\"\n+ * would prune.\n+ */\n+size_t rerere_gc_estimate(struct repository *r, size_t limit);\n+\n #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, \"rerere-autoupdate\", (v), \\\n \tN_(\"update the index with reused conflict resolution if possible\"))\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 5fbb16f0f0..4f65fa9439 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -1016,37 +1016,70 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' '\n \ttest_expect_rerere_gc git maintenance run --task=rerere-gc\n '\n \n-test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '\n+test_expect_success 'rerere-gc task with --auto only prunes with stale entries' '\n \ttest_when_finished \"rm -rf .git/rr-cache\" &&\n+\tentry_1=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) &&\n+\tentry_2=.git/rr-cache/172$(echo $ZERO_OID | cut -c4-) &&\n+\tentry_3=.git/rr-cache/173$(echo $ZERO_OID | cut -c4-) &&\n+\n+\t# Without the \"rr-cache\" directory there is nothing to prune.\n \t! git maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&\n-\tmkdir .git/rr-cache &&\n+\n+\t# Fresh unresolved entries are not stale.\n+\tfor e in $entry_1 $entry_2 $entry_3\n+\tdo\n+\t\tmkdir -p $e &&\n+\t\techo preimage >$e/preimage || return 1\n+\tdone &&\n \t! git maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&\n-\t: >.git/rr-cache/entry &&\n+\n+\t# Entries are sampled using the \"17\" prefix, so we scale up the\n+\t# estimate by 256. A single entry is not sufficient to reach the\n+\t# default limit of 512.\n+\ttest-tool chmtime =-$((16 * 86400)) $entry_1/preimage &&\n+\t! git maintenance is-needed --auto --task=rerere-gc &&\n+\n+\t# A second prunable entry will reach the limit though and will thus get\n+\t# pruned.\n+\ttest-tool chmtime =-$((16 * 86400)) $entry_2/preimage &&\n \tgit maintenance is-needed --auto --task=rerere-gc &&\n-\ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc\n+\n+\t# The prunable entries are gone, the other one remains.\n+\ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc &&\n+\ttest_path_is_missing $entry_1 &&\n+\ttest_path_is_missing $entry_2 &&\n+\ttest_path_is_dir $entry_3\n '\n \n test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.auto' '\n \ttest_when_finished \"rm -rf .git/rr-cache\" &&\n+\tentry=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) &&\n \n \t# A negative value should always prune.\n \tgit -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&\n \n-\t# A positive value prunes when there is at least one entry.\n-\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n-\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n-\tmkdir .git/rr-cache &&\n-\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n-\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n-\t: >.git/rr-cache/entry-1 &&\n-\tgit -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n-\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n+\t# A positive value prunes only when the estimated number of stale\n+\t# entries is at least as big. A single sampled entry counts for 256\n+\t# estimated entries.\n+\tmkdir -p $entry &&\n+\techo preimage >$entry/preimage &&\n+\ttest-tool chmtime =-$((16 * 86400)) $entry/preimage &&\n+\n+\t! git -c maintenance.rerere-gc.auto=257 maintenance is-needed --auto --task=rerere-gc &&\n+\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=257 maintenance run --auto --task=rerere-gc &&\n+\ttest_path_is_dir $entry &&\n+\n+\tgit -c maintenance.rerere-gc.auto=256 maintenance is-needed --auto --task=rerere-gc &&\n+\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=256 maintenance run --auto --task=rerere-gc &&\n+\ttest_path_is_missing $entry &&\n \n \t# Zero should never prune.\n-\t: >.git/rr-cache/entry-1 &&\n+\tmkdir -p $entry &&\n+\techo preimage >$entry/preimage &&\n+\ttest-tool chmtime =-$((16 * 86400)) $entry/preimage &&\n \t! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc\n '\n\n-- \n2.55.0.979.g7e5102b832.dirty\n\n"},{"id":"551858","messageId":"CAA0xjtoUzHf0bGzydZ9PR=sgz=mdA=4pDDkHr5uJhv9fT9ShsA@mail.gmail.com","threadId":"66262","inReplyTo":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im","subject":"Re: [PATCH 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Thomas Bachem","fromEmail":"mail@thomasbachem.com","sentAt":"2026-09-03T12:12:11Z","receivedAt":"2026-09-03T12:12:24Z","isPatch":true,"body":"Hi Patrick,\n\nOn Thu, Sep 03, 2026 at 11:04:56AM +0200, Patrick Steinhardt wrote:\n> as reported and discussed in [1]. Thanks!\n\nThanks for the quick turnaround. I built the series on 3cb9185f65,\nt4200 and t7900 pass, and it does what the commit message says: with\nthe default of 512, a single stale entry in the sample is not enough\nand two are, and fresh entries never trigger it however many there\nare.\n\nThat takes the gc out of my repro, for two reasons: its 20000 entries\nare fresh, and their names come from a counter rather than a hash, so\nnone starts with 17 and the sampler never sees them. With 5000 real\nhashes as names, all older than the cutoff, \"git maintenance\nis-needed --auto --task=rerere-gc\" reports it as needed again, so the\nrepro in v2 will look like that.\n\nTested-by: Thomas Bachem \n\nThanks,\nTom\n"},{"id":"551870","messageId":"a63c3bbe-28b8-4026-9c07-11c2d445c504@gmail.com","threadId":"66262","inReplyTo":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-1-9929c45a9788@pks.im","subject":"Re: [PATCH 1/2] rerere: extract logic to determine whether entries are stale","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-09-03T14:11:20Z","receivedAt":"2026-09-03T14:11:25Z","isPatch":true,"body":"On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:\n> When garbage collecting rerere entries we need to figure out whether any\n> given entry is stale before pruning it. In a subsequent commit we're\n> about to introduce a second caller that wants to determine staleness,\n> but the logic is not currently reusable.\n> \n> Extract the logic to compute staleness by introducing two new helper\n> functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.\n\nThanks for doing these extractions. It reduces complexity in the top-\nlevel logic.\n\n> -static void prune_one(struct rerere_id *id,\n> -\t\t timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)\n...> +static bool rerere_id_is_stale(struct rerere_id *id,\n> +\t\t\t timestamp_t cutoff_resolve,\n> +\t\t\t timestamp_t cutoff_noresolve)\n\nThis modification of prune_one() to a staleness check is good to\nhave split, but...\n\n> \t\tfor (id.variant = 0, id.collection = rr_dir;\n> \t\t id.variant < id.collection->status_nr;\n> \t\t id.variant++) {\n> -\t\t\tprune_one(&id, cutoff_resolve, cutoff_noresolve);\n> +\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))\n> +\t\t\t\tunlink_rr_item(&id);\n> \t\t\tif (id.collection->status[id.variant])\n> \t\t\t\tnow_empty = 0;\n> \t\t}\n\n...this loop gets slightly more complicated. This is not worth\na change, but I'm thinking out loud that I would have updated\nprune_one to be this simple:\n\nstatic void prune_one(struct rerere_id *id,\n\t\t timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)\n{\n\tif (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))\n\t\tunlink_rr_item(&id);\n} \nand left the loop alone. This is only a preference, as your\nimplementation is also quite clean.\n\nI did look to patch 2 to see if this choice of splitting the\nprune_one() method had an impact there, and it doesn't appear\nto matter.\n\nThe rerere_gc_cutoffs() and rerere_id_is_stale() methods are\nneeded in patch 2, so this adjustment to prune_one() is\nimportant.\n\nThanks,\n-Stolee\n"},{"id":"551871","messageId":"2ca2b4db-1fd9-46e8-9385-260a12af43bb@gmail.com","threadId":"66262","inReplyTo":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-2-9929c45a9788@pks.im","subject":"Re: [PATCH 2/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-09-03T14:19:33Z","receivedAt":"2026-09-03T14:19:43Z","isPatch":true,"body":"On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:\n> The \"rerere-gc\" maintenance task is responsible for pruning rerere\n> entries older than a certain configurable cutoff point. Whether or not\n> the task gets run during auto-maintenance can be configured via\n> \"maintenance.rerere-gc.auto\":\n> \n> - A negative value indicates that maintenance should always run.\n> \n> - A zero value indicates that maintenance should never run.\n> \n> - Otherwise, a positive value indicates that maintenance should always\n> run in case we have at least a single rerere entry.\n> \n> While the first two conditions are sensible, the last one is less so as\n> it does not account for whether we would even prune old entries in the\n> first place. Instead, it effectively implies that we unconditionally\n> spawn \"git rerere gc\" when rerere is enabled. Chances are high though\n> that there is nothing to prune, as the default cutoff dates are 60 days\n> for resolved rerere entries and 15 days for unresolved ones.\n\nI agree on these points. \n> @@ -121,10 +121,10 @@ maintenance.rerere-gc.auto::\n> \tThis integer config option controls how often the `rerere-gc` task\n> \tshould be run as part of `git maintenance run --auto`. If zero, then\n> \tthe `rerere-gc` task will not run with the `--auto` option. A negative\n> -\tvalue will force the task to run every time. Otherwise, any positive\n> -\tvalue implies the command will run when the \"rr-cache\" directory exists\n> -\tand has at least one entry, regardless of whether it is stale or not.\n> -\tThis heuristic may be refined in the future. The default value is 1.\n> +\tvalue will force the task to run every time. Otherwise, a positive\n> +\tvalue implies the command should run when the estimated number of stale\n> +\tentries that would be pruned is greater than or equal to the configured\n> +\tvalue. The default value is 512.\n\nThanks for updating the docs so clearly.\n> maintenance.worktree-prune.auto::\n> \tThis integer config option controls how often the `worktree-prune` task\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index de2f9e7fed..9147418a61 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -396,31 +396,13 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,\n> \n> static int rerere_gc_condition(struct gc_config *cfg UNUSED)\n> {\n> -\tstruct strbuf path = STRBUF_INIT;\n> -\tint should_gc = 0, limit = 1;\n> -\tDIR *dir = NULL;\n> +\tint limit = 512;\n> \n> \trepo_config_get_int(the_repository, \"maintenance.rerere-gc.auto\", &limit);\n> +\tif (limit <= 0)\n> +\t\treturn limit < 0;\n\nThis is cute, but works. It's logically equivalent to\n\n\tif (!limit)\n\t\treturn 0;\n\tif (limit < 0)\n\t\treturn 1;\n\nwhich would map more directly to the two documented cases. It takes\nthe slightest amount of mental processing to connect the docs to\nthe format you have.\n\n> +\treturn rerere_gc_estimate(the_repository, limit) >= (size_t)limit;\n> }\n\nI do like that this method is simpler in the builtin code in favor\nof a method that has access to rerere internals.\n\nI do wonder if rerere_gc_estimate() should be\nrerere_stale_above_limit() instead, as we are not using any callers\nthat care about the resulting number other than \"is it at least limit?\"\n\n> +size_t rerere_gc_estimate(struct repository *r, size_t limit)\n> +{\n> +\ttimestamp_t cutoff_resolve, cutoff_noresolve;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tstruct dirent *e;\n> +\tsize_t count = 0;\n> +\tDIR *dir;\n> +\n> +\tdir = opendir(repo_git_path_replace(r, &buf, \"rr-cache\"));\n> +\tif (!dir)\n> +\t\tgoto out;\n> +\n> +\trerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);\n> +\n> +\twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n> +\t\tstruct rerere_id id;\n> +\n> +\t\t/*\n> +\t\t * We estimate the number of stale entries by only considering\n> +\t\t * those starting with \"17\". This is the same strategy that we\n> +\t\t * use for estimating the number of loose objects.\n> +\t\t */\n> +\t\tif (!starts_with(e->d_name, \"17\") ||\n> +\t\t !is_rr_cache_dirname(e->d_name))\n> +\t\t\tcontinue;\n> +\n> +\t\tid.collection = find_rerere_dir(e->d_name);\n> +\t\tfor (id.variant = 0;\n> +\t\t id.variant < id.collection->status_nr;\n> +\t\t id.variant++) {\n> +\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve,\n> +\t\t\t\t\t cutoff_noresolve)) {\n> +\t\t\t\tcount += 256;\n> +\t\t\t\tif (count >= limit)\n> +\t\t\t\t\tgoto out;\n\nThis short-circuit is valuable and helps me understand the method\nprototype including a limit. If the method is changed to be a\nboolean result, then this would be 'result 1; goto out;'\n\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +out:\n> +\tif (dir)\n> +\t\tclosedir(dir);\n> +\tfree_rerere_dirs();\n> +\tstrbuf_release(&buf);\n> +\treturn count;\n> +}\n> +\n\nAgain, all I can find are taste preferences. This is a good\nimplementation and leaves some flexibility for future callers to\ncare about the number of stale entries.\n\nThank you also, for covering your change with tests.\n\nBoth patches LGTM.\n\nThanks,\n-Stolee\n"},{"id":"551926","messageId":"appVS2P6ThaRU9fc@pks.im","threadId":"66262","inReplyTo":"a63c3bbe-28b8-4026-9c07-11c2d445c504@gmail.com","subject":"Re: [PATCH 1/2] rerere: extract logic to determine whether entries are stale","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-04T05:21:15Z","receivedAt":"2026-09-04T05:21:30Z","isPatch":true,"body":"On Thu, Sep 03, 2026 at 10:11:20AM -0400, Derrick Stolee wrote:\n> On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:\n> > When garbage collecting rerere entries we need to figure out whether any\n> > given entry is stale before pruning it. In a subsequent commit we're\n> > about to introduce a second caller that wants to determine staleness,\n> > but the logic is not currently reusable.\n> > \n> > Extract the logic to compute staleness by introducing two new helper\n> > functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.\n> \n> Thanks for doing these extractions. It reduces complexity in the top-\n> level logic.\n> \n> > -static void prune_one(struct rerere_id *id,\n> > -\t\t timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)\n> ...> +static bool rerere_id_is_stale(struct rerere_id *id,\n> > +\t\t\t timestamp_t cutoff_resolve,\n> > +\t\t\t timestamp_t cutoff_noresolve)\n> \n> This modification of prune_one() to a staleness check is good to\n> have split, but...\n> \n> > \t\tfor (id.variant = 0, id.collection = rr_dir;\n> > \t\t id.variant < id.collection->status_nr;\n> > \t\t id.variant++) {\n> > -\t\t\tprune_one(&id, cutoff_resolve, cutoff_noresolve);\n> > +\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))\n> > +\t\t\t\tunlink_rr_item(&id);\n> > \t\t\tif (id.collection->status[id.variant])\n> > \t\t\t\tnow_empty = 0;\n> > \t\t}\n> \n> ...this loop gets slightly more complicated. This is not worth\n> a change, but I'm thinking out loud that I would have updated\n> prune_one to be this simple:\n> \n> static void prune_one(struct rerere_id *id,\n> \t\t timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)\n> {\n> \tif (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))\n> \t\tunlink_rr_item(&id);\n> } \n> and left the loop alone. This is only a preference, as your\n> implementation is also quite clean.\n\nThat's fair. I originally retained `prune_one()`, but then I wasn't sure\nwhether it's really worth it anymore given that it's essentially a\ntwo-line function now. Anyway, will restore it.\n\nPatrick\n"},{"id":"551927","messageId":"appVVYr6oW0fyMRD@pks.im","threadId":"66262","inReplyTo":"2ca2b4db-1fd9-46e8-9385-260a12af43bb@gmail.com","subject":"Re: [PATCH 2/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-04T05:21:25Z","receivedAt":"2026-09-04T05:21:36Z","isPatch":true,"body":"On Thu, Sep 03, 2026 at 10:19:33AM -0400, Derrick Stolee wrote:\n> On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:\n> > diff --git a/builtin/gc.c b/builtin/gc.c\n> > index de2f9e7fed..9147418a61 100644\n> > --- a/builtin/gc.c\n> > +++ b/builtin/gc.c\n> > @@ -396,31 +396,13 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,\n> > \n> > static int rerere_gc_condition(struct gc_config *cfg UNUSED)\n> > {\n> > -\tstruct strbuf path = STRBUF_INIT;\n> > -\tint should_gc = 0, limit = 1;\n> > -\tDIR *dir = NULL;\n> > +\tint limit = 512;\n> > \n> > \trepo_config_get_int(the_repository, \"maintenance.rerere-gc.auto\", &limit);\n> > +\tif (limit <= 0)\n> > +\t\treturn limit < 0;\n> \n> This is cute, but works. It's logically equivalent to\n> \n> \tif (!limit)\n> \t\treturn 0;\n> \tif (limit < 0)\n> \t\treturn 1;\n> \n> which would map more directly to the two documented cases. It takes\n> the slightest amount of mental processing to connect the docs to\n> the format you have.\n\nThis also existed in the preimage, but I agree it's harder to read than\nreally necessary. Will improve while at it.\n\n> > +\treturn rerere_gc_estimate(the_repository, limit) >= (size_t)limit;\n> > }\n> \n> I do like that this method is simpler in the builtin code in favor\n> of a method that has access to rerere internals.\n> \n> I do wonder if rerere_gc_estimate() should be\n> rerere_stale_above_limit() instead, as we are not using any callers\n> that care about the resulting number other than \"is it at least limit?\"\n\nAgreed, the current name isn't great. I'm somehow hestitant to use\n`rerere_stale_above_limit()` too, though. I'll adapt it to\n`rerere_gc_needed()` instead.\n\nThanks for your review!\n\nPatrick\n"},{"id":"551929","messageId":"20260904-b4-pks-maintenance-rerere-gc-heuristic-v2-0-b1691121fe1c@pks.im","threadId":"66262","inReplyTo":"20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im","subject":"[PATCH v2 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-04T07:03:04Z","receivedAt":"2026-09-04T07:03:28Z","isPatch":true,"body":"Hi,\n\nas reported and discussed in [1]. Thanks!\n\nChanges in v2:\n - Restore `prune_one()`.\n - Handle \"maintenance.rerere-gc.auto\" values explicitly.\n - Rename `rerere_gc_estimate()` to `rerere_gc_needed()`.\n - Link to v1: https://patch.msgid.link/20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im\n\nPatrick\n\n[1]: \n\n---\nPatrick Steinhardt (2):\n rerere: extract logic to determine whether entries are stale\n builtin/maintenance: improve heuristic for \"rerere gc\"\n\n Documentation/config/maintenance.adoc | 8 +--\n builtin/gc.c | 28 +++--------\n rerere.c | 94 ++++++++++++++++++++++++++++++-----\n rerere.h | 6 +++\n t/t7900-maintenance.sh | 61 +++++++++++++++++------\n 5 files changed, 144 insertions(+), 53 deletions(-)\n\nRange-diff versus v1:\n\n1: 343dbf1c0c ! 1: 1b0b7a7b9a rerere: extract logic to determine whether entries are stale\n @@ rerere.c: static void unlink_rr_item(struct rerere_id *id)\n \t\tcutoff = cutoff_noresolve;\n \t}\n -\tif (then < cutoff)\n --\t\tunlink_rr_item(id);\n +\n +\treturn then < cutoff;\n ++}\n ++\n ++static void prune_one(struct rerere_id *id,\n ++\t\t timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)\n ++{\n ++\tif (rerere_id_is_stale(id, cutoff_resolve, cutoff_noresolve))\n + \t\tunlink_rr_item(id);\n }\n \n - /* Does the basename in \"path\" look plausibly like an rr-cache entry? */\n @@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)\n \tDIR *dir;\n \tstruct dirent *e;\n @@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)\n \trepo_config(the_repository, git_default_config, NULL);\n \tdir = opendir(repo_git_path_replace(the_repository, &buf, \"rr-cache\"));\n \tif (!dir)\n -@@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)\n - \t\tfor (id.variant = 0, id.collection = rr_dir;\n - \t\t id.variant < id.collection->status_nr;\n - \t\t id.variant++) {\n --\t\t\tprune_one(&id, cutoff_resolve, cutoff_noresolve);\n -+\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))\n -+\t\t\t\tunlink_rr_item(&id);\n - \t\t\tif (id.collection->status[id.variant])\n - \t\t\t\tnow_empty = 0;\n - \t\t}\n2: c8a52f0663 ! 2: 1ceb798cdf builtin/maintenance: improve heuristic for \"rerere gc\"\n @@ builtin/gc.c: static int maintenance_task_rerere_gc(struct maintenance_run_opts\n -\tif (!dir)\n -\t\tgoto out;\n -\tshould_gc = !!readdir_skip_dot_and_dotdot(dir);\n -+\tif (limit <= 0)\n -+\t\treturn limit < 0;\n ++\tif (!limit)\n ++\t\treturn 0; /* never prune */\n ++\tif (limit < 0)\n ++\t\treturn 1; /* always prune */\n \n -out:\n -\tstrbuf_release(&path);\n -\tif (dir)\n -\t\tclosedir(dir);\n -\treturn should_gc;\n -+\treturn rerere_gc_estimate(the_repository, limit) >= (size_t)limit;\n ++\treturn rerere_gc_needed(the_repository, (size_t)limit);\n }\n \n #define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \\\n @@ rerere.c: static int is_rr_cache_dirname(const char *path)\n \treturn !parse_oid_hex(path, &oid, &end) && !*end;\n }\n \n -+size_t rerere_gc_estimate(struct repository *r, size_t limit)\n ++bool rerere_gc_needed(struct repository *r, size_t limit)\n +{\n +\ttimestamp_t cutoff_resolve, cutoff_noresolve;\n +\tstruct strbuf buf = STRBUF_INIT;\n ++\tbool needed = false;\n +\tstruct dirent *e;\n +\tsize_t count = 0;\n +\tDIR *dir;\n @@ rerere.c: static int is_rr_cache_dirname(const char *path)\n +\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve,\n +\t\t\t\t\t cutoff_noresolve)) {\n +\t\t\t\tcount += 256;\n -+\t\t\t\tif (count >= limit)\n ++\t\t\t\tif (count >= limit) {\n ++\t\t\t\t\tneeded = true;\n +\t\t\t\t\tgoto out;\n ++\t\t\t\t}\n +\t\t\t}\n +\t\t}\n +\t}\n @@ rerere.c: static int is_rr_cache_dirname(const char *path)\n +\t\tclosedir(dir);\n +\tfree_rerere_dirs();\n +\tstrbuf_release(&buf);\n -+\treturn count;\n ++\treturn needed;\n +}\n +\n void rerere_gc(struct repository *r, struct string_list *rr)\n @@ rerere.h: int rerere_remaining(struct repository *, struct string_list *);\n void rerere_gc(struct repository *, struct string_list *);\n \n +/*\n -+ * Estimate the number of stale entries that a run of \"git rerere gc\"\n -+ * would prune.\n ++ * Check whether garbage collection for rerere entries is needed, which is\n ++ * the case when there's at least `limit` stale entries that would be pruned.\n + */\n -+size_t rerere_gc_estimate(struct repository *r, size_t limit);\n ++bool rerere_gc_needed(struct repository *r, size_t limit);\n +\n #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, \"rerere-autoupdate\", (v), \\\n \tN_(\"update the index with reused conflict resolution if possible\"))\n\n---\nbase-commit: 3cb9185f65410273787f74333cc027d2ea5daada\nchange-id: 20260903-b4-pks-maintenance-rerere-gc-heuristic-763b0a9a50d2\n\n"},{"id":"551930","messageId":"20260904-b4-pks-maintenance-rerere-gc-heuristic-v2-1-b1691121fe1c@pks.im","threadId":"66262","inReplyTo":"20260904-b4-pks-maintenance-rerere-gc-heuristic-v2-0-b1691121fe1c@pks.im","subject":"[PATCH v2 1/2] rerere: extract logic to determine whether entries are stale","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-04T07:03:05Z","receivedAt":"2026-09-04T07:03:30Z","isPatch":true,"body":"When garbage collecting rerere entries we need to figure out whether any\ngiven entry is stale before pruning it. In a subsequent commit we're\nabout to introduce a second caller that wants to determine staleness,\nbut the logic is not currently reusable.\n\nExtract the logic to compute staleness by introducing two new helper\nfunctions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.\n\nSigned-off-by: Patrick Steinhardt \n---\n rerere.c | 44 +++++++++++++++++++++++++++++++-------------\n 1 file changed, 31 insertions(+), 13 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 3d3bd0db16..073422dbf3 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -1173,22 +1173,44 @@ static void unlink_rr_item(struct rerere_id *id)\n \tstrbuf_release(&buf);\n }\n \n-static void prune_one(struct rerere_id *id,\n-\t\t timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)\n+static void rerere_gc_cutoffs(struct repository *r,\n+\t\t\t timestamp_t *cutoff_resolve,\n+\t\t\t timestamp_t *cutoff_noresolve)\n+{\n+\ttimestamp_t now = time(NULL);\n+\n+\tif (repo_config_get_expiry_in_days(r, \"gc.rerereresolved\",\n+\t\t\t\t\t cutoff_resolve, now))\n+\t\t*cutoff_resolve = now - 60 * 86400;\n+\tif (repo_config_get_expiry_in_days(r, \"gc.rerereunresolved\",\n+\t\t\t\t\t cutoff_noresolve, now))\n+\t\t*cutoff_noresolve = now - 15 * 86400;\n+}\n+\n+static bool rerere_id_is_stale(struct rerere_id *id,\n+\t\t\t timestamp_t cutoff_resolve,\n+\t\t\t timestamp_t cutoff_noresolve)\n {\n \ttimestamp_t then;\n \ttimestamp_t cutoff;\n \n \tthen = rerere_last_used_at(id);\n-\tif (then)\n+\tif (then) {\n \t\tcutoff = cutoff_resolve;\n-\telse {\n+\t} else {\n \t\tthen = rerere_created_at(id);\n \t\tif (!then)\n-\t\t\treturn;\n+\t\t\treturn false;\n \t\tcutoff = cutoff_noresolve;\n \t}\n-\tif (then < cutoff)\n+\n+\treturn then < cutoff;\n+}\n+\n+static void prune_one(struct rerere_id *id,\n+\t\t timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)\n+{\n+\tif (rerere_id_is_stale(id, cutoff_resolve, cutoff_noresolve))\n \t\tunlink_rr_item(id);\n }\n \n@@ -1206,18 +1228,14 @@ void rerere_gc(struct repository *r, struct string_list *rr)\n \tDIR *dir;\n \tstruct dirent *e;\n \tint i;\n-\ttimestamp_t now = time(NULL);\n-\ttimestamp_t cutoff_noresolve = now - 15 * 86400;\n-\ttimestamp_t cutoff_resolve = now - 60 * 86400;\n+\ttimestamp_t cutoff_noresolve;\n+\ttimestamp_t cutoff_resolve;\n \tstruct strbuf buf = STRBUF_INIT;\n \n \tif (setup_rerere(r, rr, 0) < 0)\n \t\treturn;\n \n-\trepo_config_get_expiry_in_days(the_repository, \"gc.rerereresolved\",\n-\t\t\t\t &cutoff_resolve, now);\n-\trepo_config_get_expiry_in_days(the_repository, \"gc.rerereunresolved\",\n-\t\t\t\t &cutoff_noresolve, now);\n+\trerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);\n \trepo_config(the_repository, git_default_config, NULL);\n \tdir = opendir(repo_git_path_replace(the_repository, &buf, \"rr-cache\"));\n \tif (!dir)\n\n-- \n2.55.0.1007.g17ff1f9808.dirty\n\n"},{"id":"551931","messageId":"20260904-b4-pks-maintenance-rerere-gc-heuristic-v2-2-b1691121fe1c@pks.im","threadId":"66262","inReplyTo":"20260904-b4-pks-maintenance-rerere-gc-heuristic-v2-0-b1691121fe1c@pks.im","subject":"[PATCH v2 2/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-04T07:03:06Z","receivedAt":"2026-09-04T07:03:31Z","isPatch":true,"body":"The \"rerere-gc\" maintenance task is responsible for pruning rerere\nentries older than a certain configurable cutoff point. Whether or not\nthe task gets run during auto-maintenance can be configured via\n\"maintenance.rerere-gc.auto\":\n\n - A negative value indicates that maintenance should always run.\n\n - A zero value indicates that maintenance should never run.\n\n - Otherwise, a positive value indicates that maintenance should always\n run in case we have at least a single rerere entry.\n\nWhile the first two conditions are sensible, the last one is less so as\nit does not account for whether we would even prune old entries in the\nfirst place. Instead, it effectively implies that we unconditionally\nspawn \"git rerere gc\" when rerere is enabled. Chances are high though\nthat there is nothing to prune, as the default cutoff dates are 60 days\nfor resolved rerere entries and 15 days for unresolved ones.\n\nBesides being a waste of compute, it also obstructs concurrent processes\nthat want to write new resolutions as garbage collection takes a central\nlock file, as reported in [1]. That race is a longstanding one that\nexisted even before we introduced fine-grained maintenance tasks, and\nthe proper fix is to use a locking timeout in the writing processes. But\nthe race is made worse by us performing garbage collection a lot more\noften.\n\nRefine the heuristic to take into account whether any entries can be\npruned in the first place. This ensures that we'll only ever run this\ntask in situations where it will do anything, and should thus result in\na lot less frequent invocations of \"git rerere gc\".\n\nFurthermore, tweak the meaning of \"maintenance.rerere-gc.auto\" so that\npositive values allow the user to configure the number of prunable\nentries that need to exist before we run it and set the default value to\n512. This number is pulled out of thin air, but it ensures that we know\nto batch-delete entries instead of pruning every single entry that is\nolder than the cutoff point.\n\nNote that this now requires us to actually open the rerere-entry\ndirectories and stat the individual files in there, which does add a bit\nof overhead when one has lots of rerere entries. To counteract this\noverhead, we thus use the same sampling heuristic as we do for loose\nobjects, where we only consider those entries that start with a \"17\".\n\n[1]: \n\nReported-by: Thomas Bachem \nSigned-off-by: Patrick Steinhardt \n---\n Documentation/config/maintenance.adoc | 8 ++---\n builtin/gc.c | 28 ++++------------\n rerere.c | 50 ++++++++++++++++++++++++++++\n rerere.h | 6 ++++\n t/t7900-maintenance.sh | 61 +++++++++++++++++++++++++++--------\n 5 files changed, 113 insertions(+), 40 deletions(-)\n\ndiff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc\nindex da8be9f812..77977dcc48 100644\n--- a/Documentation/config/maintenance.adoc\n+++ b/Documentation/config/maintenance.adoc\n@@ -121,10 +121,10 @@ maintenance.rerere-gc.auto::\n \tThis integer config option controls how often the `rerere-gc` task\n \tshould be run as part of `git maintenance run --auto`. If zero, then\n \tthe `rerere-gc` task will not run with the `--auto` option. A negative\n-\tvalue will force the task to run every time. Otherwise, any positive\n-\tvalue implies the command will run when the \"rr-cache\" directory exists\n-\tand has at least one entry, regardless of whether it is stale or not.\n-\tThis heuristic may be refined in the future. The default value is 1.\n+\tvalue will force the task to run every time. Otherwise, a positive\n+\tvalue implies the command should run when the estimated number of stale\n+\tentries that would be pruned is greater than or equal to the configured\n+\tvalue. The default value is 512.\n \n maintenance.worktree-prune.auto::\n \tThis integer config option controls how often the `worktree-prune` task\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex de2f9e7fed..57a3520263 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -396,31 +396,15 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,\n \n static int rerere_gc_condition(struct gc_config *cfg UNUSED)\n {\n-\tstruct strbuf path = STRBUF_INIT;\n-\tint should_gc = 0, limit = 1;\n-\tDIR *dir = NULL;\n+\tint limit = 512;\n \n \trepo_config_get_int(the_repository, \"maintenance.rerere-gc.auto\", &limit);\n-\tif (limit <= 0) {\n-\t\tshould_gc = limit < 0;\n-\t\tgoto out;\n-\t}\n-\n-\t/*\n-\t * We skip garbage collection in case we either have no \"rr-cache\"\n-\t * directory or when it doesn't contain at least one entry.\n-\t */\n-\trepo_git_path_replace(the_repository, &path, \"rr-cache\");\n-\tdir = opendir(path.buf);\n-\tif (!dir)\n-\t\tgoto out;\n-\tshould_gc = !!readdir_skip_dot_and_dotdot(dir);\n+\tif (!limit)\n+\t\treturn 0; /* never prune */\n+\tif (limit < 0)\n+\t\treturn 1; /* always prune */\n \n-out:\n-\tstrbuf_release(&path);\n-\tif (dir)\n-\t\tclosedir(dir);\n-\treturn should_gc;\n+\treturn rerere_gc_needed(the_repository, (size_t)limit);\n }\n \n #define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \\\ndiff --git a/rerere.c b/rerere.c\nindex 073422dbf3..1c3745d9e3 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -1222,6 +1222,56 @@ static int is_rr_cache_dirname(const char *path)\n \treturn !parse_oid_hex(path, &oid, &end) && !*end;\n }\n \n+bool rerere_gc_needed(struct repository *r, size_t limit)\n+{\n+\ttimestamp_t cutoff_resolve, cutoff_noresolve;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tbool needed = false;\n+\tstruct dirent *e;\n+\tsize_t count = 0;\n+\tDIR *dir;\n+\n+\tdir = opendir(repo_git_path_replace(r, &buf, \"rr-cache\"));\n+\tif (!dir)\n+\t\tgoto out;\n+\n+\trerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);\n+\n+\twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n+\t\tstruct rerere_id id;\n+\n+\t\t/*\n+\t\t * We estimate the number of stale entries by only considering\n+\t\t * those starting with \"17\". This is the same strategy that we\n+\t\t * use for estimating the number of loose objects.\n+\t\t */\n+\t\tif (!starts_with(e->d_name, \"17\") ||\n+\t\t !is_rr_cache_dirname(e->d_name))\n+\t\t\tcontinue;\n+\n+\t\tid.collection = find_rerere_dir(e->d_name);\n+\t\tfor (id.variant = 0;\n+\t\t id.variant < id.collection->status_nr;\n+\t\t id.variant++) {\n+\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve,\n+\t\t\t\t\t cutoff_noresolve)) {\n+\t\t\t\tcount += 256;\n+\t\t\t\tif (count >= limit) {\n+\t\t\t\t\tneeded = true;\n+\t\t\t\t\tgoto out;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+out:\n+\tif (dir)\n+\t\tclosedir(dir);\n+\tfree_rerere_dirs();\n+\tstrbuf_release(&buf);\n+\treturn needed;\n+}\n+\n void rerere_gc(struct repository *r, struct string_list *rr)\n {\n \tstruct string_list to_remove = STRING_LIST_INIT_DUP;\ndiff --git a/rerere.h b/rerere.h\nindex d4b5f7c932..feeb0e2c9f 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -39,6 +39,12 @@ int rerere_remaining(struct repository *, struct string_list *);\n void rerere_clear(struct repository *, struct string_list *);\n void rerere_gc(struct repository *, struct string_list *);\n \n+/*\n+ * Check whether garbage collection for rerere entries is needed, which is\n+ * the case when there's at least `limit` stale entries that would be pruned.\n+ */\n+bool rerere_gc_needed(struct repository *r, size_t limit);\n+\n #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, \"rerere-autoupdate\", (v), \\\n \tN_(\"update the index with reused conflict resolution if possible\"))\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 5fbb16f0f0..4f65fa9439 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -1016,37 +1016,70 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' '\n \ttest_expect_rerere_gc git maintenance run --task=rerere-gc\n '\n \n-test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '\n+test_expect_success 'rerere-gc task with --auto only prunes with stale entries' '\n \ttest_when_finished \"rm -rf .git/rr-cache\" &&\n+\tentry_1=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) &&\n+\tentry_2=.git/rr-cache/172$(echo $ZERO_OID | cut -c4-) &&\n+\tentry_3=.git/rr-cache/173$(echo $ZERO_OID | cut -c4-) &&\n+\n+\t# Without the \"rr-cache\" directory there is nothing to prune.\n \t! git maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&\n-\tmkdir .git/rr-cache &&\n+\n+\t# Fresh unresolved entries are not stale.\n+\tfor e in $entry_1 $entry_2 $entry_3\n+\tdo\n+\t\tmkdir -p $e &&\n+\t\techo preimage >$e/preimage || return 1\n+\tdone &&\n \t! git maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&\n-\t: >.git/rr-cache/entry &&\n+\n+\t# Entries are sampled using the \"17\" prefix, so we scale up the\n+\t# estimate by 256. A single entry is not sufficient to reach the\n+\t# default limit of 512.\n+\ttest-tool chmtime =-$((16 * 86400)) $entry_1/preimage &&\n+\t! git maintenance is-needed --auto --task=rerere-gc &&\n+\n+\t# A second prunable entry will reach the limit though and will thus get\n+\t# pruned.\n+\ttest-tool chmtime =-$((16 * 86400)) $entry_2/preimage &&\n \tgit maintenance is-needed --auto --task=rerere-gc &&\n-\ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc\n+\n+\t# The prunable entries are gone, the other one remains.\n+\ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc &&\n+\ttest_path_is_missing $entry_1 &&\n+\ttest_path_is_missing $entry_2 &&\n+\ttest_path_is_dir $entry_3\n '\n \n test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.auto' '\n \ttest_when_finished \"rm -rf .git/rr-cache\" &&\n+\tentry=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) &&\n \n \t# A negative value should always prune.\n \tgit -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&\n \n-\t# A positive value prunes when there is at least one entry.\n-\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n-\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n-\tmkdir .git/rr-cache &&\n-\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n-\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n-\t: >.git/rr-cache/entry-1 &&\n-\tgit -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n-\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n+\t# A positive value prunes only when the estimated number of stale\n+\t# entries is at least as big. A single sampled entry counts for 256\n+\t# estimated entries.\n+\tmkdir -p $entry &&\n+\techo preimage >$entry/preimage &&\n+\ttest-tool chmtime =-$((16 * 86400)) $entry/preimage &&\n+\n+\t! git -c maintenance.rerere-gc.auto=257 maintenance is-needed --auto --task=rerere-gc &&\n+\ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=257 maintenance run --auto --task=rerere-gc &&\n+\ttest_path_is_dir $entry &&\n+\n+\tgit -c maintenance.rerere-gc.auto=256 maintenance is-needed --auto --task=rerere-gc &&\n+\ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=256 maintenance run --auto --task=rerere-gc &&\n+\ttest_path_is_missing $entry &&\n \n \t# Zero should never prune.\n-\t: >.git/rr-cache/entry-1 &&\n+\tmkdir -p $entry &&\n+\techo preimage >$entry/preimage &&\n+\ttest-tool chmtime =-$((16 * 86400)) $entry/preimage &&\n \t! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc\n '\n\n-- \n2.55.0.1007.g17ff1f9808.dirty\n\n"},{"id":"551962","messageId":"15a488b2-b4ae-4ac8-8cb3-f06ef5bbb52b@gmail.com","threadId":"66262","inReplyTo":"20260904-b4-pks-maintenance-rerere-gc-heuristic-v2-0-b1691121fe1c@pks.im","subject":"Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-09-04T13:51:59Z","receivedAt":"2026-09-04T13:52:01Z","isPatch":true,"body":"On 9/4/2026 3:03 AM, Patrick Steinhardt wrote:\n\n> Range-diff versus v1:\n\nThank you for taking the time to respond to my nitpicks. I think\nthe end result is a bit cleaner, and the patches have some more\nclarity, too. \n> 1: 343dbf1c0c ! 1: 1b0b7a7b9a rerere: extract logic to determine whether entries are stale\n> -@@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)\n> - \t\tfor (id.variant = 0, id.collection = rr_dir;\n> - \t\t id.variant < id.collection->status_nr;\n> - \t\t id.variant++) {\n> --\t\t\tprune_one(&id, cutoff_resolve, cutoff_noresolve);\n> -+\t\t\tif (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))\n> -+\t\t\t\tunlink_rr_item(&id);\n> - \t\t\tif (id.collection->status[id.variant])\n> - \t\t\t\tnow_empty = 0;\n> - \t\t}\n\nI like that this diff is no longer in the patch. Thanks!\n\n> 2: c8a52f0663 ! 2: 1ceb798cdf builtin/maintenance: improve heuristic for \"rerere gc\"\n\n> -+\tif (limit <= 0)\n> -+\t\treturn limit < 0;\n> ++\tif (!limit)\n> ++\t\treturn 0; /* never prune */\n> ++\tif (limit < 0)\n> ++\t\treturn 1; /* always prune */\n\nThe extra comments are helpful here, too!\n\n> -+\treturn rerere_gc_estimate(the_repository, limit) >= (size_t)limit;\n> ++\treturn rerere_gc_needed(the_repository, (size_t)limit);\n\nThis looks much cleaner, thanks!\n\nThis version LGTM.\n-Stolee\n"},{"id":"551967","messageId":"xmqqfqzp6pir.fsf@gitster.g","threadId":"66262","inReplyTo":"20260904-b4-pks-maintenance-rerere-gc-heuristic-v2-0-b1691121fe1c@pks.im","subject":"Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-04T14:48:44Z","receivedAt":"2026-09-04T14:48:47Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> Hi,\n>\n> as reported and discussed in [1]. Thanks!\n\nCan you, and everybody else, refrain from forcing all readers to\nvisit a different message to understand what it is? It does not\nhelp that [1] is a full description of both problem and solution\nthat is not designed to be a summary to begin with, and to add\ninsult to injury, it is AI slop wall of text that mistakenly thinks\nthat more is better.\n\nPerhaps you could have distilled the essense down to several lines?\n\n Since Git 2.54, background maintenance triggers after a commit\n runs \"git rerere gc\", which acquires the MERGE_RR.lock. During\n rebase, a subsequent sequencer commit also tries to acquire this\n lock within milliseconds. Due to use of LOCK_DIE_ON_ERROR,\n whichever arrives second aborts, causing rebase failures.\n\nI'll leave it as an exercise to readers to summarize the solution\npart that this series (not the original one) proposes to make.\n\n> Changes in v2:\n> - Restore `prune_one()`.\n> - Handle \"maintenance.rerere-gc.auto\" values explicitly.\n> - Rename `rerere_gc_estimate()` to `rerere_gc_needed()`.\n> - Link to v1: https://patch.msgid.link/20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im\n\nI find that all the changes between v1 and v2 that came as response\nto Derrick's review highly valuable. The \"cute\" expression is gone\nand the result is much easier to read ;-).\n\nThanks.\n"},{"id":"551982","messageId":"xmqqld9h56yt.fsf@gitster.g","threadId":"66262","inReplyTo":"xmqqfqzp6pir.fsf@gitster.g","subject":"Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-04T16:14:50Z","receivedAt":"2026-09-04T16:14:54Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> Patrick Steinhardt writes:\n>\n>> Hi,\n>>\n>> as reported and discussed in [1]. Thanks!\n>\n> Can you, and everybody else, refrain from forcing all readers to\n> visit a different message to understand what it is? It does not\n> help that [1] is a full description of both problem and solution\n> that is not designed to be a summary to begin with, and to add\n> insult to injury, it is AI slop wall of text that mistakenly thinks\n> that more is better.\n>\n> Perhaps you could have distilled the essense down to several lines?\n>\n> Since Git 2.54, background maintenance triggers after a commit\n> runs \"git rerere gc\", which acquires the MERGE_RR.lock. During\n> rebase, a subsequent sequencer commit also tries to acquire this\n> lock within milliseconds. Due to use of LOCK_DIE_ON_ERROR,\n> whichever arrives second aborts, causing rebase failures.\n>\n> I'll leave it as an exercise to readers to summarize the solution\n> part that this series (not the original one) proposes to make.\n\nHmph.\n\nSo the two-patch series is not about what happens when two \"rerere\ngc\" trigger in quick successions, and even with the \"improve\"d\nheuristic, the second \"rerere gc\" would fail the same way when when\nanother one is already running?\n"},{"id":"551988","messageId":"CAA0xjtrL8DJp61jp7s0L6L+RviwQz=-PEo7qZvCTh+8nT2cdfw@mail.gmail.com","threadId":"66262","inReplyTo":"xmqqld9h56yt.fsf@gitster.g","subject":"Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Thomas Bachem","fromEmail":"mail@thomasbachem.com","sentAt":"2026-09-04T16:53:59Z","receivedAt":"2026-09-04T16:54:13Z","isPatch":true,"body":"Hi Junio,\n\nOn 04/09/2026 18:14, Junio C Hamano wrote:\n> So the two-patch series is not about what happens when two \"rerere\n> gc\" trigger in quick successions, and even with the \"improve\"d\n> heuristic, the second \"rerere gc\" would fail the same way when when\n> another one is already running?\n\nRight, Patrick's series only makes the gc run less often. The lock\nitself is the subject of\n\n [PATCH v3] rerere: keep a background gc from killing a rebase\n \n\nwhere setup_rerere() waits rerere.lockTimeout for it and then goes on\nwithout rerere, and a gc that finds it held gives up at once.\n\nThomas\n"},{"id":"552091","messageId":"ap5WeY7o2dmAIn2B@pks.im","threadId":"66262","inReplyTo":"CAA0xjtrL8DJp61jp7s0L6L+RviwQz=-PEo7qZvCTh+8nT2cdfw@mail.gmail.com","subject":"Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-07T06:15:21Z","receivedAt":"2026-09-07T06:15:27Z","isPatch":true,"body":"On Fri, Sep 04, 2026 at 06:53:59PM +0200, Thomas Bachem wrote:\n> Hi Junio,\n> \n> On 04/09/2026 18:14, Junio C Hamano wrote:\n> > So the two-patch series is not about what happens when two \"rerere\n> > gc\" trigger in quick successions, and even with the \"improve\"d\n> > heuristic, the second \"rerere gc\" would fail the same way when when\n> > another one is already running?\n> \n> Right, Patrick's series only makes the gc run less often. The lock\n> itself is the subject of\n> \n> [PATCH v3] rerere: keep a background gc from killing a rebase\n> \n> \n> where setup_rerere() waits rerere.lockTimeout for it and then goes on\n> without rerere, and a gc that finds it held gives up at once.\n\nYes, exactly. This is really two issues:\n\n - rerere cannot handle concurrent writes at all, and will die\n immediately when somebody else has taken the lock. This is a\n long-standing issue, and should be fixed via Thomas' series that\n introduces a timeout for the lock.\n\n - The heuristic for garbage collecting rerere entries is way too\n trigger-friendly, which wastes resources and makes the above issue\n more likely to trigger.\n\nSo in the end, we want to have both patch series merged to address the\nissue from both ends.\n\nThanks!\n\nPatrick\n"},{"id":"552092","messageId":"ap5WiLQDzqdSyzvI@pks.im","threadId":"66262","inReplyTo":"xmqqfqzp6pir.fsf@gitster.g","subject":"Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for \"rerere gc\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-07T06:15:36Z","receivedAt":"2026-09-07T06:15:41Z","isPatch":true,"body":"On Fri, Sep 04, 2026 at 07:48:44AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > Hi,\n> >\n> > as reported and discussed in [1]. Thanks!\n> \n> Can you, and everybody else, refrain from forcing all readers to\n> visit a different message to understand what it is? It does not\n> help that [1] is a full description of both problem and solution\n> that is not designed to be a summary to begin with, and to add\n> insult to injury, it is AI slop wall of text that mistakenly thinks\n> that more is better.\n> \n> Perhaps you could have distilled the essense down to several lines?\n> \n> Since Git 2.54, background maintenance triggers after a commit\n> runs \"git rerere gc\", which acquires the MERGE_RR.lock. During\n> rebase, a subsequent sequencer commit also tries to acquire this\n> lock within milliseconds. Due to use of LOCK_DIE_ON_ERROR,\n> whichever arrives second aborts, causing rebase failures.\n> \n> I'll leave it as an exercise to readers to summarize the solution\n> part that this series (not the original one) proposes to make.\n\nFair, will adapt going forward.\n\nPatrick\n"}]}