Merge branch 'ps/tune-rerere-gc' into next

"git maintenance" triggered "rerere gc" in unappropriate times and
interfered with "git rebase" etc. too much.  The conditions "rerere
gc" gets triggered have been tweaked.

* ps/tune-rerere-gc:
  builtin/maintenance: improve heuristic for "rerere gc"
  rerere: extract logic to determine whether entries are stale

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

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;
-	}
+	if (!limit)
+		return 0; /* never prune */
+	if (limit < 0)
+		return 1; /* always prune */
 
-	/*
-	 * 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);
-
-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) \

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);
 }
 
@@ -1200,24 +1222,70 @@ 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;
 	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)

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

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
 '