Re: [PATCH 2/2] rerere: fix overeager gc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



SZEDER Gábor <szeder@xxxxxxxxxx> writes:

> On Mon, Jul 12, 2010 at 05:40:11PM -0700, Junio C Hamano wrote:
>> SZEDER Gábor <szeder@xxxxxxxxxx> writes:
>> > +static time_t rerere_last_used_at(const char *name)
>> > +{
>> > +	struct stat st;
>> > +	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
>> > +}
>> 
>> Doesn't has_rerere_resolution() already do a stat on this path?  There are
>> only two allers of the function so it would probably make sense to pass a
>> pointer to struct stat from the caller to avoid one extra call to stat.
>
> rerere_last_used_at() returns 0 when the stat() on 'postimage' fails,
> exactly like has_rerere_resolution().  Consequently, we can use
> rerere_last_used_at() to determine whether a resolution exists, too.

"When was this last used?" is answered with "infinitely long time ago" for
an item that does not exist.  Between the ones that have never been
resolved (i.e. no "postimage") and the ones that have not been used for a
long time, there is no material difference in the expiration, as long as
"long time" exceeds rerere-unresolved.  Ok; I can see the logic in that.

I wonder if swapping the order of things to check may make it easier to
read, though.  If it was used, we want to compare gc.rerereresolved with
that timestamp, and otherwise we want to compare gc.rerereunresolved with
the timestamp of the creation.  I.e. something like this...

 builtin/rerere.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/rerere.c b/builtin/rerere.c
index 7e45afe..6d1b580 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -59,16 +59,16 @@ static void garbage_collect(struct string_list *rr)
 	while ((e = readdir(dir))) {
 		if (is_dot_or_dotdot(e->d_name))
 			continue;
-		then = rerere_created_at(e->d_name);
-		if (!then)
-			continue;
-		if (has_rerere_resolution(e->d_name)) {
-			then = rerere_last_used_at(e->d_name);
+
+		then = rerere_last_used_at(e->d_name);
+		if (then) {
+			cutoff = cutoff_resolve;
+		} else {
+			then = rerere_created_at(e->d_name);
 			if (!then)
 				continue;
-			cutoff = cutoff_resolve;
-		} else
 			cutoff = cutoff_noresolve;
+		}
 		if (then < now - cutoff * 86400)
 			string_list_append(e->d_name, &to_remove);
 	}


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]