preserve mtime of local clone

PATCH In review Clemens Buchacher 4 participants 13 messages Jun 15, 2016

Share & tools

Clemens Buchacher

author

2016-06-15 22:47 UTC #0 lore raw

PATCH 1/1 preserve mtime of local clone

A local clone without hardlinks copies all objects, including dangling ones, to the new repository. Since the mtimes are renewed, those dangling objects cannot be pruned by "git gc --prune", even if they would have been old enough for pruning in the original repository.

Instead, preserve mtime during copy. "git gc --prune" will then work in the clone just like it would have in the original.

Signed-off-by Clemens Buchacher <[email protected]>

---

I noticed this problem when I cloned a repo with lots of old dangling objects onto a windows machine. git-gui immediately recommended running git-gc, and I did. But each time I restarted git-gui, it recommended git-gc again, because there were still plenty of dangling objects lying around which could not be removed due to their recent mtimes.

So there is actually a problem with git-gui's recommendation. Especially on Windows, where it only checks for 1 or more files in .git/objects/42 (as opposed to 8 files on other platforms). The probability of that happening if the repo contains about 100 loose objects is 1-(254/255)^100 = 32%. The probability for the same to happen with at least 2 files is only 6% [*]. Maybe that would be a good compromise?

Alternatively, git-gc could remember the number of dangling objects, and git-gui can adjust its recommendation accordingly, taking that number and the date of the lastest repack into account.

Clemens

[*] The following octave script shows the probability for m or more objects to be in .git/objects/42 for a total of n objects.

m = [1 2 8];
n = 100:100:3000;

P = zeros(length(n), length(m));
for k = 1:length(n)

	P(n(k), :) = 1-binocdf(m-1, n(k), 1/255);

end
plot(n, P);

n \ m 1 2 8
100 32% 6% 0%
500 86% 58% 0%
1000 98% 90% 5%
2000 100% 100% 55%

---
 builtin-clone.c   |    2 +-
 builtin-init-db.c |    2 +-
 cache.h           |    6 ++++--
 copy.c            |   25 ++++++++++++++++++++++---
 lockfile.c        |    2 +-
 rerere.c          |    2 +-
 6 files changed, 30 insertions(+), 9 deletions(-)

builtin-clone.c +1 -1

index ad04808..cb3c895 100644
@@ -269,7 +269,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
 				die_errno("failed to create link '%s'", dest->buf);
 			option_no_hardlinks = 1;
 		}
-		if (copy_file(dest->buf, src->buf, 0666))
+		if (copy_file(dest->buf, src->buf, 0666, 1))
 			die_errno("failed to copy file to '%s'", dest->buf);
 	}
 	closedir(dir);

builtin-init-db.c +1 -1

index dd84cae..5deb81d 100644
@@ -100,7 +100,7 @@ static void copy_templates_1(char *path, int baselen,
 				die_errno("cannot symlink '%s' '%s'", lnk, path);
 		}
 		else if (S_ISREG(st_template.st_mode)) {
-			if (copy_file(path, template, st_template.st_mode))
+			if (copy_file(path, template, st_template.st_mode, 0))
 				die_errno("cannot copy '%s' to '%s'", template,
 					  path);
 		}

cache.h +4 -2

index 5fad24c..1875c97 100644
@@ -921,8 +921,10 @@ extern const char *git_mailmap_file;
 
 /* IO helper functions */
 extern void maybe_flush_or_die(FILE *, const char *);
-extern int copy_fd(int ifd, int ofd);
-extern int copy_file(const char *dst, const char *src, int mode);
+extern int copy_fd(int ifd, int ofd, int preserve_times);
+extern int copy_file(const char *dst, const char *src, int mode, int
+		preserve_times);
+extern int copy_times(int ofd, int ifd);
 extern ssize_t read_in_full(int fd, void *buf, size_t count);
 extern ssize_t write_in_full(int fd, const void *buf, size_t count);
 extern void write_or_die(int fd, const void *buf, size_t count);

copy.c +22 -3

index e54d15a..fe0380e 100644
@@ -1,6 +1,6 @@
 #include "cache.h"
 
-int copy_fd(int ifd, int ofd)
+int copy_fd(int ifd, int ofd, int preserve_times)
 {
 	while (1) {
 		char buffer[8192];

@@ -31,11 +31,18 @@ int copy_fd(int ifd, int ofd)
 			}
 		}
 	}
+	if (preserve_times && copy_times(ofd, ifd)) {
+		int time_error = errno;
+		close(ifd);
+		return error("copy-fd: failed to preserve times: %s",
+				strerror(time_error));
+	}
 	close(ifd);
 	return 0;
 }
 
-int copy_file(const char *dst, const char *src, int mode)
+int copy_file(const char *dst, const char *src, int mode,
+		int preserve_times)
 {
 	int fdi, fdo, status;

@@ -46,7 +53,7 @@ int copy_file(const char *dst, const char *src, int mode)
 		close(fdi);
 		return fdo;
 	}
-	status = copy_fd(fdi, fdo);
+	status = copy_fd(fdi, fdo, preserve_times);
 	if (close(fdo) != 0)
 		return error("%s: close error: %s", dst, strerror(errno));

@@ -55,3 +62,15 @@ int copy_file(const char *dst, const char *src, int mode)
 
 	return status;
 }
+
+int copy_times(int ofd, int ifd)
+{
+	struct stat st;
+	struct timespec times[2];
+	if (fstat(ifd, &st))
+		return -1;
+	times[0].tv_nsec = UTIME_OMIT;
+	times[1].tv_sec = st.st_mtime;
+	times[1].tv_nsec = ST_MTIME_NSEC(st);
+	return futimens(ofd, times);
+}

lockfile.c +1 -1

index eb931ed..c7bbd4d 100644
@@ -196,7 +196,7 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
 			close(fd);
 			return error("cannot open '%s' for copying", path);
 		}
-	} else if (copy_fd(orig_fd, fd)) {
+	} else if (copy_fd(orig_fd, fd, 0)) {
 		if (flags & LOCK_DIE_ON_ERROR)
 			exit(128);
 		close(fd);

rerere.c +1 -1

index 87360dc..d25f5f1 100644
@@ -326,7 +326,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 			continue;
 
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
-		copy_file(rerere_path(name, "postimage"), path, 0666);
+		copy_file(rerere_path(name, "postimage"), path, 0666, 0);
 	mark_resolved:
 		rr->items[i].util = NULL;
 	}

Signature · 1 line

1.6.4.2.266.gbaa17

Junio C Hamano

2016-06-15 22:47 UTC #1 lore raw

copy.c int copy_file(const char *dst, const char *src, int mode) · line 76

Quoted 11 lines · Clemens Buchacher

copy.c +11 -0

+int copy_times(int ofd, int ifd)
+{
+	struct stat st;
+	struct timespec times[2];
+	if (fstat(ifd, &st))
+		return -1;
+	times[0].tv_nsec = UTIME_OMIT;
+	times[1].tv_sec = st.st_mtime;
+	times[1].tv_nsec = ST_MTIME_NSEC(st);
+	return futimens(ofd, times);
+}

Hmm, futimens() is relatively new. Are minority platforms folks Ok with this patch?

At least SunOS 5.11 (OpenSolaris 0811) seems to barf on UTIME_OMIT.

Clemens Buchacher

author

2016-06-15 22:47 UTC #2 lore raw

Quoted 17 lines · Junio C Hamano

copy.c int copy_file(const char *dst, const char *src, int mode) · line 76
Quoted 11 lines · Clemens Buchacher
copy.c +11 -0
+int copy_times(int ofd, int ifd)
+{
+	struct stat st;
+	struct timespec times[2];
+	if (fstat(ifd, &st))
+		return -1;
+	times[0].tv_nsec = UTIME_OMIT;
+	times[1].tv_sec = st.st_mtime;
+	times[1].tv_nsec = ST_MTIME_NSEC(st);
+	return futimens(ofd, times);
+}
Hmm, futimens() is relatively new. Are minority platforms folks Ok with this patch?

Quoted 1 line

At least SunOS 5.11 (OpenSolaris 0811) seems to barf on UTIME_OMIT.

If it's a problem we can use utime() instead. I was just trying to use the file descriptors, since they were available. But the patch would be a little smaller if I didn't touch copy_fd().

Clemens

Clemens Buchacher

author

2016-06-15 22:47 UTC #3 lore raw

PATCH 1/1 preserve mtime of local clone

Instead, preserve mtime during copy. "git gc --prune" will then work in the clone just like it did in the original.

Signed-off-by Clemens Buchacher <[email protected]>

---

Quoted 3 lines · Clemens Buchacher

If it's a problem we can use utime() instead. I was just trying to use the file descriptors, since they were available. But the patch would be a little smaller if I didn't touch copy_fd().

Here we go.

 builtin-clone.c   |    2 +-
 builtin-init-db.c |    2 +-
 cache.h           |    4 +++-
 copy.c            |   18 +++++++++++++++++-
 rerere.c          |    2 +-
 5 files changed, 23 insertions(+), 5 deletions(-)

builtin-clone.c +1 -1

index ad04808..cb3c895 100644
@@ -269,7 +269,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
 				die_errno("failed to create link '%s'", dest->buf);
 			option_no_hardlinks = 1;
 		}
-		if (copy_file(dest->buf, src->buf, 0666))
+		if (copy_file(dest->buf, src->buf, 0666, 1))
 			die_errno("failed to copy file to '%s'", dest->buf);
 	}
 	closedir(dir);

builtin-init-db.c +1 -1

index dd84cae..5deb81d 100644
@@ -100,7 +100,7 @@ static void copy_templates_1(char *path, int baselen,
 				die_errno("cannot symlink '%s' '%s'", lnk, path);
 		}
 		else if (S_ISREG(st_template.st_mode)) {
-			if (copy_file(path, template, st_template.st_mode))
+			if (copy_file(path, template, st_template.st_mode, 0))
 				die_errno("cannot copy '%s' to '%s'", template,
 					  path);
 		}

cache.h +3 -1

index 5fad24c..ac692d0 100644
@@ -922,7 +922,9 @@ extern const char *git_mailmap_file;
 /* IO helper functions */
 extern void maybe_flush_or_die(FILE *, const char *);
 extern int copy_fd(int ifd, int ofd);
-extern int copy_file(const char *dst, const char *src, int mode);
+extern int copy_file(const char *dst, const char *src, int mode, int
+		preserve_times);
+extern int copy_times(const char *dst, const char *src);
 extern ssize_t read_in_full(int fd, void *buf, size_t count);
 extern ssize_t write_in_full(int fd, const void *buf, size_t count);
 extern void write_or_die(int fd, const void *buf, size_t count);

copy.c +17 -1

index e54d15a..fb5e946 100644
@@ -35,7 +35,21 @@ int copy_fd(int ifd, int ofd)
 	return 0;
 }
 
-int copy_file(const char *dst, const char *src, int mode)
+int copy_times(const char *dst, const char *src)
+{
+	struct stat st;
+	struct utimbuf times;
+	if (stat(src, &st) < 0)
+		return -1;
+	times.actime = st.st_atime;
+	times.modtime = st.st_mtime;
+	if (utime(dst, &times) < 0)
+		return -1;
+	return 0;
+}
+
+int copy_file(const char *dst, const char *src, int mode,
+		int preserve_times)
 {
 	int fdi, fdo, status;

@@ -52,6 +66,8 @@ int copy_file(const char *dst, const char *src, int mode)
 
 	if (!status && adjust_shared_perm(dst))
 		return -1;
+	if (!status && preserve_times && copy_times(dst, src))
+		return -1;
 
 	return status;
 }

rerere.c +1 -1

index 87360dc..d25f5f1 100644
@@ -326,7 +326,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 			continue;
 
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
-		copy_file(rerere_path(name, "postimage"), path, 0666);
+		copy_file(rerere_path(name, "postimage"), path, 0666, 0);
 	mark_resolved:
 		rr->items[i].util = NULL;
 	}

Signature · 1 line

1.6.5.rc0.164.g5f6b0

Junio C Hamano

2016-06-15 22:47 UTC #4 lore raw

Quoted 11 lines · Clemens Buchacher

A local clone without hardlinks copies all objects, including dangling ones, to the new repository. Since the mtimes are renewed, those dangling objects cannot be pruned by "git gc --prune", even if they would have been old enough for pruning in the original repository.
Instead, preserve mtime during copy. "git gc --prune" will then work in the clone just like it did in the original.
Signed-off-by Clemens Buchacher <[email protected]>
---

Quoted 7 lines

Quoted 3 lines · Clemens Buchacher
If it's a problem we can use utime() instead. I was just trying to use the file descriptors, since they were available. But the patch would be a little smaller if I didn't touch copy_fd().
Here we go.

Thanks.

This new feature is not wanted by most of the callers of copy_file(), and it is not like they may want to conditionally pass 1 in preserve_times someday. I'd limit the damage like this replacement patch, instead.

Is it unreasonable to have a test for this one, by the way?

 builtin-clone.c |    2 +-
 cache.h         |    1 +
 copy.c          |   21 +++++++++++++++++++++
 3 files changed, 23 insertions(+), 1 deletions(-)

builtin-clone.c +1 -1

index ad04808..bab2d84 100644
@@ -269,7 +269,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
 				die_errno("failed to create link '%s'", dest->buf);
 			option_no_hardlinks = 1;
 		}
-		if (copy_file(dest->buf, src->buf, 0666))
+		if (copy_file_with_time(dest->buf, src->buf, 0666))
 			die_errno("failed to copy file to '%s'", dest->buf);
 	}
 	closedir(dir);

cache.h +1 -0

index 867918d..1668f28 100644
@@ -923,6 +923,7 @@ extern const char *git_mailmap_file;
 extern void maybe_flush_or_die(FILE *, const char *);
 extern int copy_fd(int ifd, int ofd);
 extern int copy_file(const char *dst, const char *src, int mode);
+extern int copy_file_with_time(const char *dst, const char *src, int mode);
 extern void write_or_die(int fd, const void *buf, size_t count);
 extern int write_or_whine(int fd, const void *buf, size_t count, const char *msg);
 extern int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg);

copy.c +21 -0

index e54d15a..a7f58fd 100644
@@ -35,6 +35,19 @@ int copy_fd(int ifd, int ofd)
 	return 0;
 }
 
+static int copy_times(const char *dst, const char *src)
+{
+	struct stat st;
+	struct utimbuf times;
+	if (stat(src, &st) < 0)
+		return -1;
+	times.actime = st.st_atime;
+	times.modtime = st.st_mtime;
+	if (utime(dst, &times) < 0)
+		return -1;
+	return 0;
+}
+
 int copy_file(const char *dst, const char *src, int mode)
 {
 	int fdi, fdo, status;

@@ -55,3 +68,11 @@ int copy_file(const char *dst, const char *src, int mode)
 
 	return status;
 }
+
+int copy_file_with_time(const char *dst, const char *src, int mode)
+{
+	int status = copy_file(dst, src, mode);
+	if (!status)
+		return copy_times(dst, src);
+	return status;
+}

Clemens Buchacher

author

2016-06-15 22:47 UTC #5 lore raw

PATCH v3 1/1 preserve mtime of local clone

Instead, preserve mtime during copy. "git gc --prune" will then work in the clone just like it did in the original.

Signed-off-by Clemens Buchacher <[email protected]>

---

Quoted 3 lines · Junio C Hamano

This new feature is not wanted by most of the callers of copy_file(), and it is not like they may want to conditionally pass 1 in preserve_times someday. I'd limit the damage like this replacement patch, instead.

You're right. That's much better.

Quoted 1 line

Is it unreasonable to have a test for this one, by the way?

It's not. Test added.

Clemens

 builtin-clone.c  |    2 +-
 cache.h          |    2 ++
 copy.c           |   21 +++++++++++++++++++++
 t/t5304-prune.sh |   54 +++++++++++++++++++++++++++++++++---------------------
 4 files changed, 57 insertions(+), 22 deletions(-)

builtin-clone.c +1 -1

index ad04808..bab2d84 100644
@@ -269,7 +269,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
 				die_errno("failed to create link '%s'", dest->buf);
 			option_no_hardlinks = 1;
 		}
-		if (copy_file(dest->buf, src->buf, 0666))
+		if (copy_file_with_time(dest->buf, src->buf, 0666))
 			die_errno("failed to copy file to '%s'", dest->buf);
 	}
 	closedir(dir);

cache.h +2 -0

index 5fad24c..ee9c98c 100644
@@ -923,6 +923,8 @@ extern const char *git_mailmap_file;
 extern void maybe_flush_or_die(FILE *, const char *);
 extern int copy_fd(int ifd, int ofd);
 extern int copy_file(const char *dst, const char *src, int mode);
+extern int copy_times(const char *dst, const char *src);
+extern int copy_file_with_time(const char *dst, const char *src, int mode);
 extern ssize_t read_in_full(int fd, void *buf, size_t count);
 extern ssize_t write_in_full(int fd, const void *buf, size_t count);
 extern void write_or_die(int fd, const void *buf, size_t count);

copy.c +21 -0

index e54d15a..963b481 100644
@@ -55,3 +55,24 @@ int copy_file(const char *dst, const char *src, int mode)
 
 	return status;
 }
+
+int copy_times(const char *dst, const char *src)
+{
+	struct stat st;
+	struct utimbuf times;
+	if (stat(src, &st) < 0)
+		return -1;
+	times.actime = st.st_atime;
+	times.modtime = st.st_mtime;
+	if (utime(dst, &times) < 0)
+		return -1;
+	return 0;
+}
+
+int copy_file_with_time(const char *dst, const char *src, int mode)
+{
+     int status = copy_file(dst, src, mode);
+     if (!status)
+		return copy_times(dst, src);
+     return status;
+}

t/t5304-prune.sh +33 ~~-21~~

91 lines not on this page · raw message

Signature · 1 line

1.6.5.rc0.164.g5f6b0

Clemens Buchacher

author

2016-06-15 22:47 UTC #6 lore raw

PATCH 1/1 git-gui: suggest gc only when counting at least 2 objects

In reply to Clemens Buchacher

On Windows, git-gui suggests running the garbage collector if it finds 1 or more files in .git/objects/42 (as opposed to 8 files on other platforms). The probability of that happening if the repo contains only about 100 loose objects is 32%. The probability for the same to happen with at least 2 files is only 6%, which is bit more reasonable.

The following octave script shows the probability for m or more objects to be in .git/objects/42 for a total of n objects.

m = [1 2 8];
n = 100:100:3000;

P = zeros(length(n), length(m));
for k = 1:length(n)

        P(k, :) = 1-binocdf(m-1, n(k), 1/255);

end
plot(n, P);

n \ m   1       2       8
100     32%     6%      0%
500     86%     58%     0%
1000    98%     90%     5%
2000    100%    100%    55%

Signed-off-by Clemens Buchacher <[email protected]>

---

So here is the patch for my other complaint. Note that I fixed a bug in the octave script above, in case someone wants to check the numbers.

I even tested it on a windows VM (as if it wasn't slow enough already).

Clemens

 git-gui/lib/database.tcl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

git-gui/lib/database.tcl +1 -1

index a18ac8b..44099e5 100644
@@ -91,7 +91,7 @@ proc do_fsck_objects {} {
 proc hint_gc {} {
 	set object_limit 8
 	if {[is_Windows]} {
-		set object_limit 1
+		set object_limit 2
 	}
 
 	set objects_current [llength [glob \

Signature · 1 line

1.6.5.rc0.164.g5f6b0

Junio C Hamano

2016-06-15 22:47 UTC #7 lore raw

Re: [PATCH] git-gui: suggest gc only when counting at least 2 objects

Quoted 3 lines · Clemens Buchacher

On Windows, git-gui suggests running the garbage collector if it finds 1 or more files in .git/objects/42 (as opposed to 8 files on other platforms).

Somebody cares to explain why this threashold number has to be different per platform in the first place? Instead of bumping up to 2 like your patch did, what bad things would happen if you increased it to 8 on Windows? Doesn't the same badness happen on non-Windows because they have the threashold set to 8 already?

Clemens Buchacher

author

2016-06-15 22:47 UTC #8 lore raw

Re: [PATCH] git-gui: suggest gc only when counting at least 2 objects

Quoted 2 lines · Junio C Hamano

Somebody cares to explain why this threashold number has to be different per platform in the first place?

I really don't know. I vaguely remember someone claim that performance on Windows suffered from many loose objects more than on other platforms. I can't find any discussion of it though.

Jeff King

2016-06-15 22:47 UTC #9 lore raw

Re: [PATCH] git-gui: suggest gc only when counting at least 2 objects

Quoted 7 lines · Clemens Buchacher

Quoted 2 lines · Junio C Hamano
Somebody cares to explain why this threashold number has to be different per platform in the first place?
I really don't know. I vaguely remember someone claim that performance on Windows suffered from many loose objects more than on other platforms. I can't find any discussion of it though.

Maybe 8ff487c?

-Peff

Clemens Buchacher

author

2016-06-15 22:47 UTC #10 lore raw

Re: [PATCH] git-gui: suggest gc only when counting at least 2 objects

Quoted 11 lines · Jeff King

Quoted 7 lines · Clemens Buchacher
Quoted 2 lines · Junio C Hamano
Somebody cares to explain why this threashold number has to be different per platform in the first place?
I really don't know. I vaguely remember someone claim that performance on Windows suffered from many loose objects more than on other platforms. I can't find any discussion of it though.
Maybe 8ff487c?

Ok. But it's been 2 years since then and if I'm not mistaken, there have been a number of performance improvements to msysgit. So maybe it's time to revisit that threshold.

If, on the other hand, requiring 2 objects really is too many, we should maybe check at least two or four directories, which would greatly improve the statistic.

For example, the probability of q directories containing q objects, for n objects total is

n \ q 1 4
50 18% 1%
100 32% 7%
200 54% 38%
500 86% 95%

Clemens

Clemens Buchacher

author

2016-06-15 22:47 UTC #11 lore raw

PATCH 1/1 git-gui: search 4 directories to improve statistic of gc hint

On Windows, git-gui suggests running the garbage collector if it finds 1 or more files in .git/objects/42 (as opposed to 8 files on other platforms). The probability of that happening if the repo contains about 100 loose objects is 32%. The probability for the same to happen when searching 4 directories is only 8%, which is bit more reasonable.

Also remove $objects_limit from the message, because we already know that we are above (or close to) that limit. Telling the user about that number does not really give him any useful information.

The following octave script shows the probability for at least m*q objects to be found in q subdirectories of .git/objects if n is the total number of objects.

q = 4;
m = [1 2 8];
n = 0:10:2000;

P = zeros(length(n), length(m));
for k = 1:length(n)

        P(k, :) = 1-binocdf(q*m-1, n(k), q/(256-q));

end
plot(n, P);

n \ q   1       4
50      18%     1%
100     32%     8%
200     54%     39%
500     86%     96%

Signed-off-by Clemens Buchacher <[email protected]>

---

 git-gui/lib/database.tcl |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

git-gui/lib/database.tcl +10 ~~-11~~

index a18ac8b..d4e0bed 100644
@@ -89,27 +89,26 @@ proc do_fsck_objects {} {
 }
 
 proc hint_gc {} {
-	set object_limit 8
+	set ndirs 1
+	set limit 8
 	if {[is_Windows]} {
-		set object_limit 1
+		set ndirs 4
+		set limit 1
 	}
 
-	set objects_current [llength [glob \
-		-directory [gitdir objects 42] \
+	set count [llength [glob \
 		-nocomplain \
-		-tails \
 		-- \
-		*]]
+		[gitdir objects 4\[0-[expr {$ndirs-1}]\]/*]]]
 
-	if {$objects_current >= $object_limit} {
-		set objects_current [expr {$objects_current * 250}]
-		set object_limit    [expr {$object_limit    * 250}]
+	if {$count >= $limit * $ndirs} {
+		set objects_current [expr {$count * 256/$ndirs}]
 		if {[ask_popup \
 			[mc "This repository currently has approximately %i loose objects.
 
-To maintain optimal performance it is strongly recommended that you compress the database when more than %i loose objects exist.
+To maintain optimal performance it is strongly recommended that you compress the database.
 
-Compress the database now?" $objects_current $object_limit]] eq yes} {
+Compress the database now?" $objects_current]] eq yes} {
 			do_gc
 		}
 	}

Signature · 1 line

1.6.5.rc0.164.g5f6b0

Shawn O. Pearce

2016-06-15 22:47 UTC #12 lore raw

Re: [PATCH] git-gui: suggest gc only when counting at least 2 objects

In reply to Clemens Buchacher

Quoted 11 lines · Clemens Buchacher

Quoted 10 lines · Jeff King
Quoted 7 lines · Clemens Buchacher
Quoted 2 lines · Junio C Hamano
Somebody cares to explain why this threashold number has to be different per platform in the first place?
I really don't know. I vaguely remember someone claim that performance on Windows suffered from many loose objects more than on other platforms. I can't find any discussion of it though.
Maybe 8ff487c?

Yes, it was 8ff487c. Back then I was using Windows on a daily basis and this was put into git-gui because Aunt Tillie couldn't remember do run a git-gc every so often, and performance would just drop in the bucket. It also quite a bit predates git gc --auto being sprinkled throughout the code base on various operations.

As to why its been 200 as the loose count estimate, that was just a WAG based on what I observed on my desktop. 2000 on UNIX is usually fine, 2000 on Windows meant you waited an extra 30 seconds to perform an operation.

Quoted 3 lines

Ok. But it's been 2 years since then and if I'm not mistaken, there have been a number of performance improvements to msysgit. So maybe it's time to revisit that threshold.

msysgit may have improved, but at the time I was running Git on Cygwin, and I doubt NTFS has really improved that much.

Quoted 3 lines

If, on the other hand, requiring 2 objects really is too many, we should maybe check at least two or four directories, which would greatly improve the statistic.

I'm concerned about the FS cost of checking more directories, but this is a one-time penalty on startup of git-gui so it might not be too bad if it gets us a better estimate.

Signature · 1 line

Shawn.