[PATCH 0/2] builtin/maintenance: improve heuristic for "rerere gc"

[PATCH 0/2] builtin/maintenance: improve heuristic for "rerere gc"

[Patrick Steinhardt]

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 1/2] rerere: extract logic to determine whether entries are stale

[Patrick Steinhardt]

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(-)

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

Re: [PATCH 1/2] rerere: extract logic to determine whether entries are stale

[Derrick Stolee]

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.

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 1/2] rerere: extract logic to determine whether entries are stale

[Patrick Steinhardt]

On Thu, Sep 03, 2026 at 10:11:20AM -0400, Derrick Stolee wrote:
> 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

[PATCH 2/2] builtin/maintenance: improve heuristic for "rerere gc"

[Patrick Steinhardt]

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(-)

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 2/2] builtin/maintenance: improve heuristic for "rerere gc"

[Derrick Stolee]

On 9/3/2026 5:04 AM, Patrick Steinhardt wrote:
> 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.

I agree on these points. 
> @@ -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.
>  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?"

> +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;'

> +			}
> +		}
> +	}
> +
> +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 2/2] builtin/maintenance: improve heuristic for "rerere gc"

[Patrick Steinhardt]

On Thu, Sep 03, 2026 at 10:19:33AM -0400, Derrick Stolee wrote:
> 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.

> > +	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

Re: [PATCH 0/2] builtin/maintenance: improve heuristic for "rerere gc"

[Thomas Bachem]

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

[PATCH v2 0/2] builtin/maintenance: improve heuristic for "rerere gc"

[Patrick Steinhardt]

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

[Patrick Steinhardt]

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(-)

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"

[Patrick Steinhardt]

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(-)

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"

[Derrick Stolee]

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. 
> 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"

>     -+	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"

[Junio C Hamano]

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.

> 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]

Junio C Hamano <gitster@pobox.com> writes:

> 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"

[Thomas Bachem]

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"

[Patrick Steinhardt]

On Fri, Sep 04, 2026 at 06:53:59PM +0200, Thomas Bachem wrote:
> 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"

[Patrick Steinhardt]

On Fri, Sep 04, 2026 at 07:48:44AM -0700, Junio C Hamano wrote:
> 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