patch, 2 partsrerere: extract logic to determine whether entries are stale
17 messages between Sep 3, 2026 and Sep 7, 2026, from Patrick Steinhardt, Thomas Bachem, Derrick Stolee, Junio C Hamano.
Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.
When garbage collecting rerere entries we need to figure out whether any given entry is stale before pruning it. In a subsequent commit we're about to introduce a second caller that wants to determine staleness, but the logic is not currently reusable.
Extract the logic to compute staleness by introducing two new helper functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
rerere.c | 42 +++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 15 deletions(-)
Show changes to rerere.c +27 −15
diff --git a/rerere.c b/rerere.c
index 3d3bd0db16..d01af6b71b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1173,23 +1173,38 @@ static void unlink_rr_item(struct rerere_id *id)
strbuf_release(&buf);
}
-static void prune_one(struct rerere_id *id,
- timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
+static void rerere_gc_cutoffs(struct repository *r,
+ timestamp_t *cutoff_resolve,
+ timestamp_t *cutoff_noresolve)
+{
+ timestamp_t now = time(NULL);
+
+ if (repo_config_get_expiry_in_days(r, "gc.rerereresolved",
+ cutoff_resolve, now))
+ *cutoff_resolve = now - 60 * 86400;
+ if (repo_config_get_expiry_in_days(r, "gc.rerereunresolved",
+ cutoff_noresolve, now))
+ *cutoff_noresolve = now - 15 * 86400;
+}
+
+static bool rerere_id_is_stale(struct rerere_id *id,
+ timestamp_t cutoff_resolve,
+ timestamp_t cutoff_noresolve)
{
timestamp_t then;
timestamp_t cutoff;
then = rerere_last_used_at(id);
- if (then)
+ if (then) {
cutoff = cutoff_resolve;
- else {
+ } else {
then = rerere_created_at(id);
if (!then)
- return;
+ return false;
cutoff = cutoff_noresolve;
}
- if (then < cutoff)
- unlink_rr_item(id);
+
+ return then < cutoff;
}
/* Does the basename in "path" look plausibly like an rr-cache entry? */
@@ -1206,18 +1221,14 @@ void rerere_gc(struct repository *r, struct string_list *rr)
DIR *dir;
struct dirent *e;
int i;
- timestamp_t now = time(NULL);
- timestamp_t cutoff_noresolve = now - 15 * 86400;
- timestamp_t cutoff_resolve = now - 60 * 86400;
+ timestamp_t cutoff_noresolve;
+ timestamp_t cutoff_resolve;
struct strbuf buf = STRBUF_INIT;
if (setup_rerere(r, rr, 0) < 0)
return;
- repo_config_get_expiry_in_days(the_repository, "gc.rerereresolved",
- &cutoff_resolve, now);
- repo_config_get_expiry_in_days(the_repository, "gc.rerereunresolved",
- &cutoff_noresolve, now);
+ rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
repo_config(the_repository, git_default_config, NULL);
dir = opendir(repo_git_path_replace(the_repository, &buf, "rr-cache"));
if (!dir)
@@ -1237,7 +1248,8 @@ void rerere_gc(struct repository *r, struct string_list *rr)
for (id.variant = 0, id.collection = rr_dir;
id.variant < id.collection->status_nr;
id.variant++) {
- prune_one(&id, cutoff_resolve, cutoff_noresolve);
+ if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
+ unlink_rr_item(&id);
if (id.collection->status[id.variant])
now_empty = 0;
}
--
2.55.0.979.g7e5102b832.dirty
Patrick SteinhardtSep 3, 2026, 09:04 UTC on lore[PATCH 0/2] builtin/maintenance: improve heuristic for "rerere gc"
Hi,
as reported and discussed in [1]. Thanks!
Patrick
[1]: <pull.2214.git.1788337897490.gitgitgadget@gmail.com>
---
Patrick Steinhardt (2):
rerere: extract logic to determine whether entries are stale
builtin/maintenance: improve heuristic for "rerere gc" Documentation/config/maintenance.adoc | 8 ++--
builtin/gc.c | 26 ++--------
rerere.c | 89 +++++++++++++++++++++++++++++------
rerere.h | 6 +++
t/t7900-maintenance.sh | 61 ++++++++++++++++++------
5 files changed, 135 insertions(+), 55 deletions(-)
--- base-commit: 3cb9185f65410273787f74333cc027d2ea5daada change-id: 20260903-b4-pks-maintenance-rerere-gc-heuristic-763b0a9a50d2
[PATCH 2/2] builtin/maintenance: improve heuristic for "rerere gc"
The "rerere-gc" maintenance task is responsible for pruning rerere entries older than a certain configurable cutoff point. Whether or not the task gets run during auto-maintenance can be configured via "maintenance.rerere-gc.auto":
- A negative value indicates that maintenance should always run.
- A zero value indicates that maintenance should never run.
- Otherwise, a positive value indicates that maintenance should always
run in case we have at least a single rerere entry.While the first two conditions are sensible, the last one is less so as it does not account for whether we would even prune old entries in the first place. Instead, it effectively implies that we unconditionally spawn "git rerere gc" when rerere is enabled. Chances are high though that there is nothing to prune, as the default cutoff dates are 60 days for resolved rerere entries and 15 days for unresolved ones.
Besides being a waste of compute, it also obstructs concurrent processes that want to write new resolutions as garbage collection takes a central lock file, as reported in [1]. That race is a longstanding one that existed even before we introduced fine-grained maintenance tasks, and the proper fix is to use a locking timeout in the writing processes. But the race is made worse by us performing garbage collection a lot more often.
Refine the heuristic to take into account whether any entries can be pruned in the first place. This ensures that we'll only ever run this task in situations where it will do anything, and should thus result in a lot less frequent invocations of "git rerere gc".
Furthermore, tweak the meaning of "maintenance.rerere-gc.auto" so that
positive values allow the user to configure the number of prunable
entries that need to exist before we run it and set the default value to
512. This number is pulled out of thin air, but it ensures that we know
to batch-delete entries instead of pruning every single entry that is
older than the cutoff point.
Note that this now requires us to actually open the rerere-entry directories and stat the individual files in there, which does add a bit of overhead when one has lots of rerere entries. To counteract this overhead, we thus use the same sampling heuristic as we do for loose objects, where we only consider those entries that start with a "17".
[1]: <pull.2214.git.1788337897490.gitgitgadget@gmail.com>
Reported-by: Thomas Bachem <mail@thomasbachem.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/config/maintenance.adoc | 8 ++---
builtin/gc.c | 26 +++------------
rerere.c | 47 +++++++++++++++++++++++++++
rerere.h | 6 ++++
t/t7900-maintenance.sh | 61 +++++++++++++++++++++++++++--------
5 files changed, 108 insertions(+), 40 deletions(-)
Show changes to 5 files +108 −40
Documentation/config/maintenance.adoc, builtin/gc.c, rerere.c, rerere.h, t/t7900-maintenance.sh
diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc
index da8be9f812..77977dcc48 100644
--- a/Documentation/config/maintenance.adoc
+++ b/Documentation/config/maintenance.adoc
@@ -121,10 +121,10 @@ maintenance.rerere-gc.auto::
This integer config option controls how often the `rerere-gc` task
should be run as part of `git maintenance run --auto`. If zero, then
the `rerere-gc` task will not run with the `--auto` option. A negative
- value will force the task to run every time. Otherwise, any positive
- value implies the command will run when the "rr-cache" directory exists
- and has at least one entry, regardless of whether it is stale or not.
- This heuristic may be refined in the future. The default value is 1.
+ value will force the task to run every time. Otherwise, a positive
+ value implies the command should run when the estimated number of stale
+ entries that would be pruned is greater than or equal to the configured
+ value. The default value is 512.
maintenance.worktree-prune.auto::
This integer config option controls how often the `worktree-prune` task
diff --git a/builtin/gc.c b/builtin/gc.c
index de2f9e7fed..9147418a61 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -396,31 +396,13 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,
static int rerere_gc_condition(struct gc_config *cfg UNUSED)
{
- struct strbuf path = STRBUF_INIT;
- int should_gc = 0, limit = 1;
- DIR *dir = NULL;
+ int limit = 512;
repo_config_get_int(the_repository, "maintenance.rerere-gc.auto", &limit);
- if (limit <= 0) {
- should_gc = limit < 0;
- goto out;
- }
-
- /*
- * We skip garbage collection in case we either have no "rr-cache"
- * directory or when it doesn't contain at least one entry.
- */
- repo_git_path_replace(the_repository, &path, "rr-cache");
- dir = opendir(path.buf);
- if (!dir)
- goto out;
- should_gc = !!readdir_skip_dot_and_dotdot(dir);
+ if (limit <= 0)
+ return limit < 0;
-out:
- strbuf_release(&path);
- if (dir)
- closedir(dir);
- return should_gc;
+ return rerere_gc_estimate(the_repository, limit) >= (size_t)limit;
}
#define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
diff --git a/rerere.c b/rerere.c
index d01af6b71b..87a42c4cc3 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1215,6 +1215,53 @@ static int is_rr_cache_dirname(const char *path)
return !parse_oid_hex(path, &oid, &end) && !*end;
}
+size_t rerere_gc_estimate(struct repository *r, size_t limit)
+{
+ timestamp_t cutoff_resolve, cutoff_noresolve;
+ struct strbuf buf = STRBUF_INIT;
+ struct dirent *e;
+ size_t count = 0;
+ DIR *dir;
+
+ dir = opendir(repo_git_path_replace(r, &buf, "rr-cache"));
+ if (!dir)
+ goto out;
+
+ rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
+
+ while ((e = readdir_skip_dot_and_dotdot(dir))) {
+ struct rerere_id id;
+
+ /*
+ * We estimate the number of stale entries by only considering
+ * those starting with "17". This is the same strategy that we
+ * use for estimating the number of loose objects.
+ */
+ if (!starts_with(e->d_name, "17") ||
+ !is_rr_cache_dirname(e->d_name))
+ continue;
+
+ id.collection = find_rerere_dir(e->d_name);
+ for (id.variant = 0;
+ id.variant < id.collection->status_nr;
+ id.variant++) {
+ if (rerere_id_is_stale(&id, cutoff_resolve,
+ cutoff_noresolve)) {
+ count += 256;
+ if (count >= limit)
+ goto out;
+ }
+ }
+ }
+
+out:
+ if (dir)
+ closedir(dir);
+ free_rerere_dirs();
+ strbuf_release(&buf);
+ return count;
+}
+
void rerere_gc(struct repository *r, struct string_list *rr)
{
struct string_list to_remove = STRING_LIST_INIT_DUP;
diff --git a/rerere.h b/rerere.h
index d4b5f7c932..898ebdd25a 100644
--- a/rerere.h
+++ b/rerere.h
@@ -39,6 +39,12 @@ int rerere_remaining(struct repository *, struct string_list *);
void rerere_clear(struct repository *, struct string_list *);
void rerere_gc(struct repository *, struct string_list *);
+/*
+ * Estimate the number of stale entries that a run of "git rerere gc"
+ * would prune.
+ */
+size_t rerere_gc_estimate(struct repository *r, size_t limit);
+
#define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \
N_("update the index with reused conflict resolution if possible"))
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 5fbb16f0f0..4f65fa9439 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1016,37 +1016,70 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' '
test_expect_rerere_gc git maintenance run --task=rerere-gc
'
-test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '
+test_expect_success 'rerere-gc task with --auto only prunes with stale entries' '
test_when_finished "rm -rf .git/rr-cache" &&
+ entry_1=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) &&
+ entry_2=.git/rr-cache/172$(echo $ZERO_OID | cut -c4-) &&
+ entry_3=.git/rr-cache/173$(echo $ZERO_OID | cut -c4-) &&
+
+ # Without the "rr-cache" directory there is nothing to prune.
! git maintenance is-needed --auto --task=rerere-gc &&
test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
- mkdir .git/rr-cache &&
+
+ # Fresh unresolved entries are not stale.
+ for e in $entry_1 $entry_2 $entry_3
+ do
+ mkdir -p $e &&
+ echo preimage >$e/preimage || return 1
+ done &&
! git maintenance is-needed --auto --task=rerere-gc &&
test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
- : >.git/rr-cache/entry &&
+
+ # Entries are sampled using the "17" prefix, so we scale up the
+ # estimate by 256. A single entry is not sufficient to reach the
+ # default limit of 512.
+ test-tool chmtime =-$((16 * 86400)) $entry_1/preimage &&
+ ! git maintenance is-needed --auto --task=rerere-gc &&
+
+ # A second prunable entry will reach the limit though and will thus get
+ # pruned.
+ test-tool chmtime =-$((16 * 86400)) $entry_2/preimage &&
git maintenance is-needed --auto --task=rerere-gc &&
- test_expect_rerere_gc git maintenance run --auto --task=rerere-gc
+
+ # The prunable entries are gone, the other one remains.
+ test_expect_rerere_gc git maintenance run --auto --task=rerere-gc &&
+ test_path_is_missing $entry_1 &&
+ test_path_is_missing $entry_2 &&
+ test_path_is_dir $entry_3
'
test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.auto' '
test_when_finished "rm -rf .git/rr-cache" &&
+ entry=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) &&
# A negative value should always prune.
git -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&
test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&
- # A positive value prunes when there is at least one entry.
- ! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
- test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
- mkdir .git/rr-cache &&
- ! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
- test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
- : >.git/rr-cache/entry-1 &&
- git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
- test_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
+ # A positive value prunes only when the estimated number of stale
+ # entries is at least as big. A single sampled entry counts for 256
+ # estimated entries.
+ mkdir -p $entry &&
+ echo preimage >$entry/preimage &&
+ test-tool chmtime =-$((16 * 86400)) $entry/preimage &&
+
+ ! git -c maintenance.rerere-gc.auto=257 maintenance is-needed --auto --task=rerere-gc &&
+ test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=257 maintenance run --auto --task=rerere-gc &&
+ test_path_is_dir $entry &&
+
+ git -c maintenance.rerere-gc.auto=256 maintenance is-needed --auto --task=rerere-gc &&
+ test_expect_rerere_gc git -c maintenance.rerere-gc.auto=256 maintenance run --auto --task=rerere-gc &&
+ test_path_is_missing $entry &&
# Zero should never prune.
- : >.git/rr-cache/entry-1 &&
+ mkdir -p $entry &&
+ echo preimage >$entry/preimage &&
+ test-tool chmtime =-$((16 * 86400)) $entry/preimage &&
! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&
test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc
'
--
2.55.0.979.g7e5102b832.dirty
Re: [PATCH 0/2] builtin/maintenance: improve heuristic for "rerere gc"
Hi Patrick,
On Thu, Sep 03, 2026 at 11:04:56AM +0200, Patrick Steinhardt wrote:
> as reported and discussed in [1]. Thanks!
Thanks for the quick turnaround. I built the series on 3cb9185f65, t4200 and t7900 pass, and it does what the commit message says: with the default of 512, a single stale entry in the sample is not enough and two are, and fresh entries never trigger it however many there are.
That takes the gc out of my repro, for two reasons: its 20000 entries are fresh, and their names come from a counter rather than a hash, so none starts with 17 and the sampler never sees them. With 5000 real hashes as names, all older than the cutoff, "git maintenance is-needed --auto --task=rerere-gc" reports it as needed again, so the repro in v2 will look like that.
Tested-by: Thomas Bachem <mail@thomasbachem.com>
Thanks, Tom
Re: [PATCH 1/2] rerere: extract logic to determine whether entries are stale
On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:
Show 7 quoted lines
> When garbage collecting rerere entries we need to figure out whether any
> given entry is stale before pruning it. In a subsequent commit we're
> about to introduce a second caller that wants to determine staleness,
> but the logic is not currently reusable.
>
> Extract the logic to compute staleness by introducing two new helper
> functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.
Thanks for doing these extractions. It reduces complexity in the top- level logic.
> -static void prune_one(struct rerere_id *id,
> - timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
...> +static bool rerere_id_is_stale(struct rerere_id *id,
> + timestamp_t cutoff_resolve,
> + timestamp_t cutoff_noresolve)
This modification of prune_one() to a staleness check is good to have split, but...
Show 9 quoted lines
> for (id.variant = 0, id.collection = rr_dir;
> id.variant < id.collection->status_nr;
> id.variant++) {
> - prune_one(&id, cutoff_resolve, cutoff_noresolve);
> + if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
> + unlink_rr_item(&id);
> if (id.collection->status[id.variant])
> now_empty = 0;
> }...this loop gets slightly more complicated. This is not worth a change, but I'm thinking out loud that I would have updated prune_one to be this simple:
static void prune_one(struct rerere_id *id,
timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
{
if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
unlink_rr_item(&id);
}
and left the loop alone. This is only a preference, as your
implementation is also quite clean.I did look to patch 2 to see if this choice of splitting the prune_one() method had an impact there, and it doesn't appear to matter.
The rerere_gc_cutoffs() and rerere_id_is_stale() methods are needed in patch 2, so this adjustment to prune_one() is important.
Thanks, -Stolee
Re: [PATCH 2/2] builtin/maintenance: improve heuristic for "rerere gc"
On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:
Show 18 quoted lines
> The "rerere-gc" maintenance task is responsible for pruning rerere
> entries older than a certain configurable cutoff point. Whether or not
> the task gets run during auto-maintenance can be configured via
> "maintenance.rerere-gc.auto":
>
> - A negative value indicates that maintenance should always run.
>
> - A zero value indicates that maintenance should never run.
>
> - Otherwise, a positive value indicates that maintenance should always
> run in case we have at least a single rerere entry.
>
> While the first two conditions are sensible, the last one is less so as
> it does not account for whether we would even prune old entries in the
> first place. Instead, it effectively implies that we unconditionally
> spawn "git rerere gc" when rerere is enabled. Chances are high though
> that there is nothing to prune, as the default cutoff dates are 60 days
> for resolved rerere entries and 15 days for unresolved ones.
Show 12 quoted lines
> @@ -121,10 +121,10 @@ maintenance.rerere-gc.auto::
> This integer config option controls how often the `rerere-gc` task
> should be run as part of `git maintenance run --auto`. If zero, then
> the `rerere-gc` task will not run with the `--auto` option. A negative
> - value will force the task to run every time. Otherwise, any positive
> - value implies the command will run when the "rr-cache" directory exists
> - and has at least one entry, regardless of whether it is stale or not.
> - This heuristic may be refined in the future. The default value is 1.
> + value will force the task to run every time. Otherwise, a positive
> + value implies the command should run when the estimated number of stale
> + entries that would be pruned is greater than or equal to the configured
> + value. The default value is 512.
Thanks for updating the docs so clearly.
Show 18 quoted lines
> maintenance.worktree-prune.auto::
> This integer config option controls how often the `worktree-prune` task
> diff --git a/builtin/gc.c b/builtin/gc.c
> index de2f9e7fed..9147418a61 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -396,31 +396,13 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,
>
> static int rerere_gc_condition(struct gc_config *cfg UNUSED)
> {
> - struct strbuf path = STRBUF_INIT;
> - int should_gc = 0, limit = 1;
> - DIR *dir = NULL;
> + int limit = 512;
>
> repo_config_get_int(the_repository, "maintenance.rerere-gc.auto", &limit);
> + if (limit <= 0)
> + return limit < 0;This is cute, but works. It's logically equivalent to
if (!limit)
return 0;
if (limit < 0)
return 1;
which would map more directly to the two documented cases. It takes the slightest amount of mental processing to connect the docs to the format you have.
> + return rerere_gc_estimate(the_repository, limit) >= (size_t)limit;
> }
I do like that this method is simpler in the builtin code in favor of a method that has access to rerere internals.
I do wonder if rerere_gc_estimate() should be rerere_stale_above_limit() instead, as we are not using any callers that care about the resulting number other than "is it at least limit?"
Show 35 quoted lines
> +size_t rerere_gc_estimate(struct repository *r, size_t limit)
> +{
> + timestamp_t cutoff_resolve, cutoff_noresolve;
> + struct strbuf buf = STRBUF_INIT;
> + struct dirent *e;
> + size_t count = 0;
> + DIR *dir;
> +
> + dir = opendir(repo_git_path_replace(r, &buf, "rr-cache"));
> + if (!dir)
> + goto out;
> +
> + rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
> +
> + while ((e = readdir_skip_dot_and_dotdot(dir))) {
> + struct rerere_id id;
> +
> + /*
> + * We estimate the number of stale entries by only considering
> + * those starting with "17". This is the same strategy that we
> + * use for estimating the number of loose objects.
> + */
> + if (!starts_with(e->d_name, "17") ||
> + !is_rr_cache_dirname(e->d_name))
> + continue;
> +
> + id.collection = find_rerere_dir(e->d_name);
> + for (id.variant = 0;
> + id.variant < id.collection->status_nr;
> + id.variant++) {
> + if (rerere_id_is_stale(&id, cutoff_resolve,
> + cutoff_noresolve)) {
> + count += 256;
> + if (count >= limit)
> + goto out;This short-circuit is valuable and helps me understand the method prototype including a limit. If the method is changed to be a boolean result, then this would be 'result 1; goto out;'
Show 12 quoted lines
> + }
> + }
> + }
> +
> +out:
> + if (dir)
> + closedir(dir);
> + free_rerere_dirs();
> + strbuf_release(&buf);
> + return count;
> +}
> +
Again, all I can find are taste preferences. This is a good implementation and leaves some flexibility for future callers to care about the number of stale entries.
Thank you also, for covering your change with tests.
Both patches LGTM.
Thanks, -Stolee
Re: [PATCH 1/2] rerere: extract logic to determine whether entries are stale
On Thu, Sep 03, 2026 at 10:11:20AM -0400, Derrick Stolee wrote:
Show 43 quoted lines
> On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:
> > When garbage collecting rerere entries we need to figure out whether any
> > given entry is stale before pruning it. In a subsequent commit we're
> > about to introduce a second caller that wants to determine staleness,
> > but the logic is not currently reusable.
> >
> > Extract the logic to compute staleness by introducing two new helper
> > functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.
>
> Thanks for doing these extractions. It reduces complexity in the top-
> level logic.
>
> > -static void prune_one(struct rerere_id *id,
> > - timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
> ...> +static bool rerere_id_is_stale(struct rerere_id *id,
> > + timestamp_t cutoff_resolve,
> > + timestamp_t cutoff_noresolve)
>
> This modification of prune_one() to a staleness check is good to
> have split, but...
>
> > for (id.variant = 0, id.collection = rr_dir;
> > id.variant < id.collection->status_nr;
> > id.variant++) {
> > - prune_one(&id, cutoff_resolve, cutoff_noresolve);
> > + if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
> > + unlink_rr_item(&id);
> > if (id.collection->status[id.variant])
> > now_empty = 0;
> > }
>
> ...this loop gets slightly more complicated. This is not worth
> a change, but I'm thinking out loud that I would have updated
> prune_one to be this simple:
>
> static void prune_one(struct rerere_id *id,
> timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
> {
> if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
> unlink_rr_item(&id);
> }
> and left the loop alone. This is only a preference, as your
> implementation is also quite clean.That's fair. I originally retained `prune_one()`, but then I wasn't sure whether it's really worth it anymore given that it's essentially a two-line function now. Anyway, will restore it.
Patrick
Re: [PATCH 2/2] builtin/maintenance: improve heuristic for "rerere gc"
On Thu, Sep 03, 2026 at 10:19:33AM -0400, Derrick Stolee wrote:
Show 28 quoted lines
> On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index de2f9e7fed..9147418a61 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -396,31 +396,13 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,
> >
> > static int rerere_gc_condition(struct gc_config *cfg UNUSED)
> > {
> > - struct strbuf path = STRBUF_INIT;
> > - int should_gc = 0, limit = 1;
> > - DIR *dir = NULL;
> > + int limit = 512;
> >
> > repo_config_get_int(the_repository, "maintenance.rerere-gc.auto", &limit);
> > + if (limit <= 0)
> > + return limit < 0;
>
> This is cute, but works. It's logically equivalent to
>
> if (!limit)
> return 0;
> if (limit < 0)
> return 1;
>
> which would map more directly to the two documented cases. It takes
> the slightest amount of mental processing to connect the docs to
> the format you have.This also existed in the preimage, but I agree it's harder to read than really necessary. Will improve while at it.
Show 9 quoted lines
> > + return rerere_gc_estimate(the_repository, limit) >= (size_t)limit;
> > }
>
> I do like that this method is simpler in the builtin code in favor
> of a method that has access to rerere internals.
>
> I do wonder if rerere_gc_estimate() should be
> rerere_stale_above_limit() instead, as we are not using any callers
> that care about the resulting number other than "is it at least limit?"
Agreed, the current name isn't great. I'm somehow hestitant to use `rerere_stale_above_limit()` too, though. I'll adapt it to `rerere_gc_needed()` instead.
Thanks for your review!
Patrick
[PATCH v2 0/2] builtin/maintenance: improve heuristic for "rerere gc"
Hi,
as reported and discussed in [1]. Thanks!
Changes in v2:
- Restore `prune_one()`.
- Handle "maintenance.rerere-gc.auto" values explicitly.
- Rename `rerere_gc_estimate()` to `rerere_gc_needed()`.
- Link to v1: https://patch.msgid.link/20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im
Patrick
[1]: <pull.2214.git.1788337897490.gitgitgadget@gmail.com>
---
Patrick Steinhardt (2):
rerere: extract logic to determine whether entries are stale
builtin/maintenance: improve heuristic for "rerere gc" Documentation/config/maintenance.adoc | 8 +--
builtin/gc.c | 28 +++--------
rerere.c | 94 ++++++++++++++++++++++++++++++-----
rerere.h | 6 +++
t/t7900-maintenance.sh | 61 +++++++++++++++++------
5 files changed, 144 insertions(+), 53 deletions(-)
Range-diff versus v1:
1: 343dbf1c0c ! 1: 1b0b7a7b9a rerere: extract logic to determine whether entries are stale
@@ rerere.c: static void unlink_rr_item(struct rerere_id *id)
cutoff = cutoff_noresolve;
}
- if (then < cutoff)
-- unlink_rr_item(id);
+
+ return then < cutoff;
++}
++
++static void prune_one(struct rerere_id *id,
++ timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
++{
++ if (rerere_id_is_stale(id, cutoff_resolve, cutoff_noresolve))
+ unlink_rr_item(id);
}
- /* Does the basename in "path" look plausibly like an rr-cache entry? */
@@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)
DIR *dir;
struct dirent *e;
@@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)
repo_config(the_repository, git_default_config, NULL);
dir = opendir(repo_git_path_replace(the_repository, &buf, "rr-cache"));
if (!dir)
-@@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)
- for (id.variant = 0, id.collection = rr_dir;
- id.variant < id.collection->status_nr;
- id.variant++) {
-- prune_one(&id, cutoff_resolve, cutoff_noresolve);
-+ if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
-+ unlink_rr_item(&id);
- if (id.collection->status[id.variant])
- now_empty = 0;
- }
2: c8a52f0663 ! 2: 1ceb798cdf builtin/maintenance: improve heuristic for "rerere gc"
@@ builtin/gc.c: static int maintenance_task_rerere_gc(struct maintenance_run_opts
- if (!dir)
- goto out;
- should_gc = !!readdir_skip_dot_and_dotdot(dir);
-+ if (limit <= 0)
-+ return limit < 0;
++ if (!limit)
++ return 0; /* never prune */
++ if (limit < 0)
++ return 1; /* always prune */
-out:
- strbuf_release(&path);
- if (dir)
- closedir(dir);
- return should_gc;
-+ return rerere_gc_estimate(the_repository, limit) >= (size_t)limit;
++ return rerere_gc_needed(the_repository, (size_t)limit);
}
#define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
@@ rerere.c: static int is_rr_cache_dirname(const char *path)
return !parse_oid_hex(path, &oid, &end) && !*end;
}
-+size_t rerere_gc_estimate(struct repository *r, size_t limit)
++bool rerere_gc_needed(struct repository *r, size_t limit)
+{
+ timestamp_t cutoff_resolve, cutoff_noresolve;
+ struct strbuf buf = STRBUF_INIT;
++ bool needed = false;
+ struct dirent *e;
+ size_t count = 0;
+ DIR *dir;
@@ rerere.c: static int is_rr_cache_dirname(const char *path)
+ if (rerere_id_is_stale(&id, cutoff_resolve,
+ cutoff_noresolve)) {
+ count += 256;
-+ if (count >= limit)
++ if (count >= limit) {
++ needed = true;
+ goto out;
++ }
+ }
+ }
+ }
@@ rerere.c: static int is_rr_cache_dirname(const char *path)
+ closedir(dir);
+ free_rerere_dirs();
+ strbuf_release(&buf);
-+ return count;
++ return needed;
+}
+
void rerere_gc(struct repository *r, struct string_list *rr)
@@ rerere.h: int rerere_remaining(struct repository *, struct string_list *);
void rerere_gc(struct repository *, struct string_list *);
+/*
-+ * Estimate the number of stale entries that a run of "git rerere gc"
-+ * would prune.
++ * Check whether garbage collection for rerere entries is needed, which is
++ * the case when there's at least `limit` stale entries that would be pruned.
+ */
-+size_t rerere_gc_estimate(struct repository *r, size_t limit);
++bool rerere_gc_needed(struct repository *r, size_t limit);
+
#define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \
N_("update the index with reused conflict resolution if possible"))--- base-commit: 3cb9185f65410273787f74333cc027d2ea5daada change-id: 20260903-b4-pks-maintenance-rerere-gc-heuristic-763b0a9a50d2
[PATCH v2 1/2] rerere: extract logic to determine whether entries are stale
When garbage collecting rerere entries we need to figure out whether any given entry is stale before pruning it. In a subsequent commit we're about to introduce a second caller that wants to determine staleness, but the logic is not currently reusable.
Extract the logic to compute staleness by introducing two new helper functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
rerere.c | 44 +++++++++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 13 deletions(-)
Show changes to rerere.c +31 −13
diff --git a/rerere.c b/rerere.c
index 3d3bd0db16..073422dbf3 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1173,22 +1173,44 @@ static void unlink_rr_item(struct rerere_id *id)
strbuf_release(&buf);
}
-static void prune_one(struct rerere_id *id,
- timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
+static void rerere_gc_cutoffs(struct repository *r,
+ timestamp_t *cutoff_resolve,
+ timestamp_t *cutoff_noresolve)
+{
+ timestamp_t now = time(NULL);
+
+ if (repo_config_get_expiry_in_days(r, "gc.rerereresolved",
+ cutoff_resolve, now))
+ *cutoff_resolve = now - 60 * 86400;
+ if (repo_config_get_expiry_in_days(r, "gc.rerereunresolved",
+ cutoff_noresolve, now))
+ *cutoff_noresolve = now - 15 * 86400;
+}
+
+static bool rerere_id_is_stale(struct rerere_id *id,
+ timestamp_t cutoff_resolve,
+ timestamp_t cutoff_noresolve)
{
timestamp_t then;
timestamp_t cutoff;
then = rerere_last_used_at(id);
- if (then)
+ if (then) {
cutoff = cutoff_resolve;
- else {
+ } else {
then = rerere_created_at(id);
if (!then)
- return;
+ return false;
cutoff = cutoff_noresolve;
}
- if (then < cutoff)
+
+ return then < cutoff;
+}
+
+static void prune_one(struct rerere_id *id,
+ timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
+{
+ if (rerere_id_is_stale(id, cutoff_resolve, cutoff_noresolve))
unlink_rr_item(id);
}
@@ -1206,18 +1228,14 @@ void rerere_gc(struct repository *r, struct string_list *rr)
DIR *dir;
struct dirent *e;
int i;
- timestamp_t now = time(NULL);
- timestamp_t cutoff_noresolve = now - 15 * 86400;
- timestamp_t cutoff_resolve = now - 60 * 86400;
+ timestamp_t cutoff_noresolve;
+ timestamp_t cutoff_resolve;
struct strbuf buf = STRBUF_INIT;
if (setup_rerere(r, rr, 0) < 0)
return;
- repo_config_get_expiry_in_days(the_repository, "gc.rerereresolved",
- &cutoff_resolve, now);
- repo_config_get_expiry_in_days(the_repository, "gc.rerereunresolved",
- &cutoff_noresolve, now);
+ rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
repo_config(the_repository, git_default_config, NULL);
dir = opendir(repo_git_path_replace(the_repository, &buf, "rr-cache"));
if (!dir)
--
2.55.0.1007.g17ff1f9808.dirty
[PATCH v2 2/2] builtin/maintenance: improve heuristic for "rerere gc"
The "rerere-gc" maintenance task is responsible for pruning rerere entries older than a certain configurable cutoff point. Whether or not the task gets run during auto-maintenance can be configured via "maintenance.rerere-gc.auto":
- A negative value indicates that maintenance should always run.
- A zero value indicates that maintenance should never run.
- Otherwise, a positive value indicates that maintenance should always
run in case we have at least a single rerere entry.While the first two conditions are sensible, the last one is less so as it does not account for whether we would even prune old entries in the first place. Instead, it effectively implies that we unconditionally spawn "git rerere gc" when rerere is enabled. Chances are high though that there is nothing to prune, as the default cutoff dates are 60 days for resolved rerere entries and 15 days for unresolved ones.
Besides being a waste of compute, it also obstructs concurrent processes that want to write new resolutions as garbage collection takes a central lock file, as reported in [1]. That race is a longstanding one that existed even before we introduced fine-grained maintenance tasks, and the proper fix is to use a locking timeout in the writing processes. But the race is made worse by us performing garbage collection a lot more often.
Refine the heuristic to take into account whether any entries can be pruned in the first place. This ensures that we'll only ever run this task in situations where it will do anything, and should thus result in a lot less frequent invocations of "git rerere gc".
Furthermore, tweak the meaning of "maintenance.rerere-gc.auto" so that
positive values allow the user to configure the number of prunable
entries that need to exist before we run it and set the default value to
512. This number is pulled out of thin air, but it ensures that we know
to batch-delete entries instead of pruning every single entry that is
older than the cutoff point.
Note that this now requires us to actually open the rerere-entry directories and stat the individual files in there, which does add a bit of overhead when one has lots of rerere entries. To counteract this overhead, we thus use the same sampling heuristic as we do for loose objects, where we only consider those entries that start with a "17".
[1]: <pull.2214.git.1788337897490.gitgitgadget@gmail.com>
Reported-by: Thomas Bachem <mail@thomasbachem.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/config/maintenance.adoc | 8 ++---
builtin/gc.c | 28 ++++------------
rerere.c | 50 ++++++++++++++++++++++++++++
rerere.h | 6 ++++
t/t7900-maintenance.sh | 61 +++++++++++++++++++++++++++--------
5 files changed, 113 insertions(+), 40 deletions(-)
Show changes to 5 files +113 −40
Documentation/config/maintenance.adoc, builtin/gc.c, rerere.c, rerere.h, t/t7900-maintenance.sh
diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc
index da8be9f812..77977dcc48 100644
--- a/Documentation/config/maintenance.adoc
+++ b/Documentation/config/maintenance.adoc
@@ -121,10 +121,10 @@ maintenance.rerere-gc.auto::
This integer config option controls how often the `rerere-gc` task
should be run as part of `git maintenance run --auto`. If zero, then
the `rerere-gc` task will not run with the `--auto` option. A negative
- value will force the task to run every time. Otherwise, any positive
- value implies the command will run when the "rr-cache" directory exists
- and has at least one entry, regardless of whether it is stale or not.
- This heuristic may be refined in the future. The default value is 1.
+ value will force the task to run every time. Otherwise, a positive
+ value implies the command should run when the estimated number of stale
+ entries that would be pruned is greater than or equal to the configured
+ value. The default value is 512.
maintenance.worktree-prune.auto::
This integer config option controls how often the `worktree-prune` task
diff --git a/builtin/gc.c b/builtin/gc.c
index de2f9e7fed..57a3520263 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -396,31 +396,15 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED,
static int rerere_gc_condition(struct gc_config *cfg UNUSED)
{
- struct strbuf path = STRBUF_INIT;
- int should_gc = 0, limit = 1;
- DIR *dir = NULL;
+ int limit = 512;
repo_config_get_int(the_repository, "maintenance.rerere-gc.auto", &limit);
- if (limit <= 0) {
- should_gc = limit < 0;
- goto out;
- }
-
- /*
- * We skip garbage collection in case we either have no "rr-cache"
- * directory or when it doesn't contain at least one entry.
- */
- repo_git_path_replace(the_repository, &path, "rr-cache");
- dir = opendir(path.buf);
- if (!dir)
- goto out;
- should_gc = !!readdir_skip_dot_and_dotdot(dir);
+ if (!limit)
+ return 0; /* never prune */
+ if (limit < 0)
+ return 1; /* always prune */
-out:
- strbuf_release(&path);
- if (dir)
- closedir(dir);
- return should_gc;
+ return rerere_gc_needed(the_repository, (size_t)limit);
}
#define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
diff --git a/rerere.c b/rerere.c
index 073422dbf3..1c3745d9e3 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1222,6 +1222,56 @@ static int is_rr_cache_dirname(const char *path)
return !parse_oid_hex(path, &oid, &end) && !*end;
}
+bool rerere_gc_needed(struct repository *r, size_t limit)
+{
+ timestamp_t cutoff_resolve, cutoff_noresolve;
+ struct strbuf buf = STRBUF_INIT;
+ bool needed = false;
+ struct dirent *e;
+ size_t count = 0;
+ DIR *dir;
+
+ dir = opendir(repo_git_path_replace(r, &buf, "rr-cache"));
+ if (!dir)
+ goto out;
+
+ rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
+
+ while ((e = readdir_skip_dot_and_dotdot(dir))) {
+ struct rerere_id id;
+
+ /*
+ * We estimate the number of stale entries by only considering
+ * those starting with "17". This is the same strategy that we
+ * use for estimating the number of loose objects.
+ */
+ if (!starts_with(e->d_name, "17") ||
+ !is_rr_cache_dirname(e->d_name))
+ continue;
+
+ id.collection = find_rerere_dir(e->d_name);
+ for (id.variant = 0;
+ id.variant < id.collection->status_nr;
+ id.variant++) {
+ if (rerere_id_is_stale(&id, cutoff_resolve,
+ cutoff_noresolve)) {
+ count += 256;
+ if (count >= limit) {
+ needed = true;
+ goto out;
+ }
+ }
+ }
+ }
+
+out:
+ if (dir)
+ closedir(dir);
+ free_rerere_dirs();
+ strbuf_release(&buf);
+ return needed;
+}
+
void rerere_gc(struct repository *r, struct string_list *rr)
{
struct string_list to_remove = STRING_LIST_INIT_DUP;
diff --git a/rerere.h b/rerere.h
index d4b5f7c932..feeb0e2c9f 100644
--- a/rerere.h
+++ b/rerere.h
@@ -39,6 +39,12 @@ int rerere_remaining(struct repository *, struct string_list *);
void rerere_clear(struct repository *, struct string_list *);
void rerere_gc(struct repository *, struct string_list *);
+/*
+ * Check whether garbage collection for rerere entries is needed, which is
+ * the case when there's at least `limit` stale entries that would be pruned.
+ */
+bool rerere_gc_needed(struct repository *r, size_t limit);
+
#define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \
N_("update the index with reused conflict resolution if possible"))
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 5fbb16f0f0..4f65fa9439 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1016,37 +1016,70 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' '
test_expect_rerere_gc git maintenance run --task=rerere-gc
'
-test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '
+test_expect_success 'rerere-gc task with --auto only prunes with stale entries' '
test_when_finished "rm -rf .git/rr-cache" &&
+ entry_1=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) &&
+ entry_2=.git/rr-cache/172$(echo $ZERO_OID | cut -c4-) &&
+ entry_3=.git/rr-cache/173$(echo $ZERO_OID | cut -c4-) &&
+
+ # Without the "rr-cache" directory there is nothing to prune.
! git maintenance is-needed --auto --task=rerere-gc &&
test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
- mkdir .git/rr-cache &&
+
+ # Fresh unresolved entries are not stale.
+ for e in $entry_1 $entry_2 $entry_3
+ do
+ mkdir -p $e &&
+ echo preimage >$e/preimage || return 1
+ done &&
! git maintenance is-needed --auto --task=rerere-gc &&
test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
- : >.git/rr-cache/entry &&
+
+ # Entries are sampled using the "17" prefix, so we scale up the
+ # estimate by 256. A single entry is not sufficient to reach the
+ # default limit of 512.
+ test-tool chmtime =-$((16 * 86400)) $entry_1/preimage &&
+ ! git maintenance is-needed --auto --task=rerere-gc &&
+
+ # A second prunable entry will reach the limit though and will thus get
+ # pruned.
+ test-tool chmtime =-$((16 * 86400)) $entry_2/preimage &&
git maintenance is-needed --auto --task=rerere-gc &&
- test_expect_rerere_gc git maintenance run --auto --task=rerere-gc
+
+ # The prunable entries are gone, the other one remains.
+ test_expect_rerere_gc git maintenance run --auto --task=rerere-gc &&
+ test_path_is_missing $entry_1 &&
+ test_path_is_missing $entry_2 &&
+ test_path_is_dir $entry_3
'
test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.auto' '
test_when_finished "rm -rf .git/rr-cache" &&
+ entry=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) &&
# A negative value should always prune.
git -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&
test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&
- # A positive value prunes when there is at least one entry.
- ! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
- test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
- mkdir .git/rr-cache &&
- ! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
- test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
- : >.git/rr-cache/entry-1 &&
- git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
- test_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
+ # A positive value prunes only when the estimated number of stale
+ # entries is at least as big. A single sampled entry counts for 256
+ # estimated entries.
+ mkdir -p $entry &&
+ echo preimage >$entry/preimage &&
+ test-tool chmtime =-$((16 * 86400)) $entry/preimage &&
+
+ ! git -c maintenance.rerere-gc.auto=257 maintenance is-needed --auto --task=rerere-gc &&
+ test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=257 maintenance run --auto --task=rerere-gc &&
+ test_path_is_dir $entry &&
+
+ git -c maintenance.rerere-gc.auto=256 maintenance is-needed --auto --task=rerere-gc &&
+ test_expect_rerere_gc git -c maintenance.rerere-gc.auto=256 maintenance run --auto --task=rerere-gc &&
+ test_path_is_missing $entry &&
# Zero should never prune.
- : >.git/rr-cache/entry-1 &&
+ mkdir -p $entry &&
+ echo preimage >$entry/preimage &&
+ test-tool chmtime =-$((16 * 86400)) $entry/preimage &&
! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&
test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc
'
--
2.55.0.1007.g17ff1f9808.dirty
Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for "rerere gc"
On 9/4/2026 3:03 AM, Patrick Steinhardt wrote:
> Range-diff versus v1:
Thank you for taking the time to respond to my nitpicks. I think the end result is a bit cleaner, and the patches have some more clarity, too.
Show 11 quoted lines
> 1: 343dbf1c0c ! 1: 1b0b7a7b9a rerere: extract logic to determine whether entries are stale
> -@@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)
> - for (id.variant = 0, id.collection = rr_dir;
> - id.variant < id.collection->status_nr;
> - id.variant++) {
> -- prune_one(&id, cutoff_resolve, cutoff_noresolve);
> -+ if (rerere_id_is_stale(&id, cutoff_resolve, cutoff_noresolve))
> -+ unlink_rr_item(&id);
> - if (id.collection->status[id.variant])
> - now_empty = 0;
> - }I like that this diff is no longer in the patch. Thanks!
> 2: c8a52f0663 ! 2: 1ceb798cdf builtin/maintenance: improve heuristic for "rerere gc"
Show 6 quoted lines
> -+ if (limit <= 0)
> -+ return limit < 0;
> ++ if (!limit)
> ++ return 0; /* never prune */
> ++ if (limit < 0)
> ++ return 1; /* always prune */
The extra comments are helpful here, too!
> -+ return rerere_gc_estimate(the_repository, limit) >= (size_t)limit;
> ++ return rerere_gc_needed(the_repository, (size_t)limit);
This looks much cleaner, thanks!
This version LGTM. -Stolee
Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for "rerere gc"
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> as reported and discussed in [1]. Thanks!
Can you, and everybody else, refrain from forcing all readers to visit a different message to understand what it is? It does not help that [1] is a full description of both problem and solution that is not designed to be a summary to begin with, and to add insult to injury, it is AI slop wall of text that mistakenly thinks that more is better.
Perhaps you could have distilled the essense down to several lines?
Since Git 2.54, background maintenance triggers after a commit
runs "git rerere gc", which acquires the MERGE_RR.lock. During
rebase, a subsequent sequencer commit also tries to acquire this
lock within milliseconds. Due to use of LOCK_DIE_ON_ERROR,
whichever arrives second aborts, causing rebase failures.I'll leave it as an exercise to readers to summarize the solution part that this series (not the original one) proposes to make.
Show 5 quoted lines
> Changes in v2:
> - Restore `prune_one()`.
> - Handle "maintenance.rerere-gc.auto" values explicitly.
> - Rename `rerere_gc_estimate()` to `rerere_gc_needed()`.
> - Link to v1: https://patch.msgid.link/20260903-b4-pks-maintenance-rerere-gc-heuristic-v1-0-9929c45a9788@pks.im
I find that all the changes between v1 and v2 that came as response to Derrick's review highly valuable. The "cute" expression is gone and the result is much easier to read ;-).
Thanks.
Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for "rerere gc"
Junio C Hamano <gitster@pobox.com> writes:
Show 23 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
>
>> Hi,
>>
>> as reported and discussed in [1]. Thanks!
>
> Can you, and everybody else, refrain from forcing all readers to
> visit a different message to understand what it is? It does not
> help that [1] is a full description of both problem and solution
> that is not designed to be a summary to begin with, and to add
> insult to injury, it is AI slop wall of text that mistakenly thinks
> that more is better.
>
> Perhaps you could have distilled the essense down to several lines?
>
> Since Git 2.54, background maintenance triggers after a commit
> runs "git rerere gc", which acquires the MERGE_RR.lock. During
> rebase, a subsequent sequencer commit also tries to acquire this
> lock within milliseconds. Due to use of LOCK_DIE_ON_ERROR,
> whichever arrives second aborts, causing rebase failures.
>
> I'll leave it as an exercise to readers to summarize the solution
> part that this series (not the original one) proposes to make.
Hmph.
So the two-patch series is not about what happens when two "rerere gc" trigger in quick successions, and even with the "improve"d heuristic, the second "rerere gc" would fail the same way when when another one is already running?
Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for "rerere gc"
Hi Junio,
On 04/09/2026 18:14, Junio C Hamano wrote:
> So the two-patch series is not about what happens when two "rerere
> gc" trigger in quick successions, and even with the "improve"d
> heuristic, the second "rerere gc" would fail the same way when when
> another one is already running?
Right, Patrick's series only makes the gc run less often. The lock itself is the subject of
[PATCH v3] rerere: keep a background gc from killing a rebase
<pull.2214.v3.git.1788537081930.gitgitgadget@gmail.com>
where setup_rerere() waits rerere.lockTimeout for it and then goes on without rerere, and a gc that finds it held gives up at once.
Thomas
Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for "rerere gc"
On Fri, Sep 04, 2026 at 06:53:59PM +0200, Thomas Bachem wrote:
Show 16 quoted lines
> Hi Junio,
>
> On 04/09/2026 18:14, Junio C Hamano wrote:
> > So the two-patch series is not about what happens when two "rerere
> > gc" trigger in quick successions, and even with the "improve"d
> > heuristic, the second "rerere gc" would fail the same way when when
> > another one is already running?
>
> Right, Patrick's series only makes the gc run less often. The lock
> itself is the subject of
>
> [PATCH v3] rerere: keep a background gc from killing a rebase
> <pull.2214.v3.git.1788537081930.gitgitgadget@gmail.com>
>
> where setup_rerere() waits rerere.lockTimeout for it and then goes on
> without rerere, and a gc that finds it held gives up at once.
Yes, exactly. This is really two issues:
- rerere cannot handle concurrent writes at all, and will die
immediately when somebody else has taken the lock. This is a
long-standing issue, and should be fixed via Thomas' series that
introduces a timeout for the lock. - The heuristic for garbage collecting rerere entries is way too
trigger-friendly, which wastes resources and makes the above issue
more likely to trigger.So in the end, we want to have both patch series merged to address the issue from both ends.
Thanks!
Patrick
Re: [PATCH v2 0/2] builtin/maintenance: improve heuristic for "rerere gc"
On Fri, Sep 04, 2026 at 07:48:44AM -0700, Junio C Hamano wrote:
Show 23 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Hi,
> >
> > as reported and discussed in [1]. Thanks!
>
> Can you, and everybody else, refrain from forcing all readers to
> visit a different message to understand what it is? It does not
> help that [1] is a full description of both problem and solution
> that is not designed to be a summary to begin with, and to add
> insult to injury, it is AI slop wall of text that mistakenly thinks
> that more is better.
>
> Perhaps you could have distilled the essense down to several lines?
>
> Since Git 2.54, background maintenance triggers after a commit
> runs "git rerere gc", which acquires the MERGE_RR.lock. During
> rebase, a subsequent sequencer commit also tries to acquire this
> lock within milliseconds. Due to use of LOCK_DIE_ON_ERROR,
> whichever arrives second aborts, causing rebase failures.
>
> I'll leave it as an exercise to readers to summarize the solution
> part that this series (not the original one) proposes to make.
Fair, will adapt going forward.
Patrick