Make "git clone" pack-fetching download statistics better

Make "git clone" pack-fetching download statistics better

[Linus Torvalds]

Make "git clone" pack-fetching download statistics better

Average it out over a few events to make the numbers stable, and fix the
silly usec->binary-ms conversion.

Yeah, yeah, it's arguably eye-candy to keep the user calm, but let's do 
that right.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

This is obviously against my previous diff to do the verbose output in the 
first place. If you want a combined diff, just holler.

This makes the download speed be totally stable for me on an otherwise 
idle DSL connection (146 kB/s, which sounds right, in case anybody cares).

diff --git a/fetch-clone.c b/fetch-clone.c
index d8216cb..da1b3ff 100644
--- a/fetch-clone.c
+++ b/fetch-clone.c
@@ -130,12 +130,35 @@ int receive_unpack_pack(int fd[2], const
 	die("git-unpack-objects died of unnatural causes %d", status);
 }
 
+/*
+ * We average out the download speed over this many "events", where
+ * an event is a minimum of about half a second. That way, we get
+ * a reasonably stable number.
+ */
+#define NR_AVERAGE (4)
+
+/*
+ * A "binary msec" is a power-of-two-msec, aka 1/1024th of a second.
+ * Keeing the time in that format means that "bytes / msecs" means
+ * is the same as kB/s (modulo rounding).
+ *
+ * 1000512 is a magic number (usecs in a second, rounded up by half
+ * of 1024, to make "rounding" come out right ;)
+ */
+#define usec_to_binarymsec(x) ((int)(x) / (1000512 >> 10))
+
 int receive_keep_pack(int fd[2], const char *me, int quiet)
 {
 	char tmpfile[PATH_MAX];
 	int ofd, ifd;
 	unsigned long total;
 	static struct timeval prev_tv;
+	struct average {
+		unsigned long bytes;
+		unsigned long time;
+	} download[NR_AVERAGE] = { {0, 0}, };
+	unsigned long avg_bytes, avg_time;
+	int idx = 0;
 
 	ifd = fd[0];
 	snprintf(tmpfile, sizeof(tmpfile),
@@ -146,6 +169,8 @@ int receive_keep_pack(int fd[2], const c
 
 	gettimeofday(&prev_tv, NULL);
 	total = 0;
+	avg_bytes = 0;
+	avg_time = 0;
 	while (1) {
 		char buf[8192];
 		ssize_t sz, wsz, pos;
@@ -184,14 +209,27 @@ int receive_keep_pack(int fd[2], const c
 			gettimeofday(&tv, NULL);
 			msecs = tv.tv_sec - prev_tv.tv_sec;
 			msecs <<= 10;
-			msecs += (int)(tv.tv_usec - prev_tv.tv_usec) >> 10;
+			msecs += usec_to_binarymsec(tv.tv_usec - prev_tv.tv_usec);
+
 			if (msecs > 500) {
 				prev_tv = tv;
 				last = total;
-				fprintf(stderr, "%4lu.%03luMB  (%lu kB/s)        \r",
+
+				/* Update averages ..*/
+				avg_bytes += diff;
+				avg_time += msecs;
+				avg_bytes -= download[idx].bytes;
+				avg_time -= download[idx].time;
+				download[idx].bytes = diff;
+				download[idx].time = msecs;
+				idx++;
+				if (idx >= NR_AVERAGE)
+					idx = 0;
+
+				fprintf(stderr, "%4lu.%03luMB  (%lu kB/s)      \r",
 					total >> 20,
 					1000*((total >> 10) & 1023)>>10,
-					diff / msecs );
+					avg_bytes / avg_time );
 			}
 		}
 	}

[PATCH] fetch-clone progress: finishing touches.

[Junio C Hamano]

This makes fetch-pack also report the progress of packing part.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * While we are doing eye-candy, this makes the silence after
   "Generating pack..." part a bit more bearable.

   Likes, dislikes, too-much's?

   BTW, don't you mean 512 down there???

        -	msecs += (int)(tv.tv_usec - prev_tv.tv_usec) >> 10;
        +	msecs += usec_to_binarymsec(tv.tv_usec - prev_tv.tv_usec);
        +
                if (msecs > 500) {
                        prev_tv = tv;

 clone-pack.c   |    4 ++--
 pack-objects.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 4 deletions(-)

21fcd1bdea2440236aea1713ea42a66bc2da5563
diff --git a/clone-pack.c b/clone-pack.c
index 719e1c4..a4370f5 100644
--- a/clone-pack.c
+++ b/clone-pack.c
@@ -125,9 +125,9 @@ static int clone_pack(int fd[2], int nr_
 	}
 	clone_handshake(fd, refs);
 
-	if (!quiet)
-		fprintf(stderr, "Generating pack ...\r");
 	status = receive_keep_pack(fd, "git-clone-pack", quiet);
+	if (!quiet)
+		fprintf(stderr, "\n");
 
 	if (!status) {
 		if (nr_match == 0)
diff --git a/pack-objects.c b/pack-objects.c
index c3f2531..2135e9a 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -3,6 +3,7 @@
 #include "delta.h"
 #include "pack.h"
 #include "csum-file.h"
+#include <sys/time.h>
 
 static const char pack_usage[] = "git-pack-objects [--non-empty] [--local] [--incremental] [--window=N] [--depth=N] {--stdout | base-name} < object-list";
 
@@ -26,6 +27,7 @@ static struct object_entry *objects = NU
 static int nr_objects = 0, nr_alloc = 0;
 static const char *base_name;
 static unsigned char pack_file_sha1[20];
+static int progress = 0;
 
 static void *delta_against(void *buf, unsigned long size, struct object_entry *entry)
 {
@@ -362,10 +364,13 @@ static void find_deltas(struct object_en
 	int i, idx;
 	unsigned int array_size = window * sizeof(struct unpacked);
 	struct unpacked *array = xmalloc(array_size);
+	int eye_candy;
 
 	memset(array, 0, array_size);
 	i = nr_objects;
 	idx = 0;
+	eye_candy = i - (nr_objects / 20);
+
 	while (--i >= 0) {
 		struct object_entry *entry = list[i];
 		struct unpacked *n = array + idx;
@@ -373,6 +378,10 @@ static void find_deltas(struct object_en
 		char type[10];
 		int j;
 
+		if (progress && i <= eye_candy) {
+			eye_candy -= nr_objects / 20;
+			fputc('.', stderr);
+		}
 		free(n->data);
 		n->entry = entry;
 		n->data = read_sha1_file(entry->sha1, type, &size);
@@ -404,11 +413,13 @@ static void prepare_pack(int window, int
 {
 	get_object_details();
 
-	fprintf(stderr, "Packing %d objects\n", nr_objects);
-
+	if (progress)
+		fprintf(stderr, "Packing %d objects", nr_objects);
 	sorted_by_type = create_sorted_list(type_size_sort);
 	if (window && depth)
 		find_deltas(sorted_by_type, window+1, depth);
+	if (progress)
+		fputc('\n', stderr);
 	write_pack_file();
 }
 
@@ -472,6 +483,10 @@ int main(int argc, char **argv)
 	int window = 10, depth = 10, pack_to_stdout = 0;
 	struct object_entry **list;
 	int i;
+	struct timeval prev_tv;
+	int eye_candy = 0;
+	int eye_candy_incr = 500;
+
 
 	setup_git_directory();
 
@@ -519,12 +534,34 @@ int main(int argc, char **argv)
 	if (pack_to_stdout != !base_name)
 		usage(pack_usage);
 
+	progress = isatty(2);
+
 	prepare_packed_git();
+	if (progress) {
+		fprintf(stderr, "Generating pack...\n");
+		gettimeofday(&prev_tv, NULL);
+	}
 	while (fgets(line, sizeof(line), stdin) != NULL) {
 		unsigned int hash;
 		char *p;
 		unsigned char sha1[20];
 
+		if (progress && (eye_candy <= nr_objects)) {
+			fprintf(stderr, "Counting objects...%d\r", nr_objects);
+			if (eye_candy && (50 <= eye_candy_incr)) {
+				struct timeval tv;
+				int time_diff;
+				gettimeofday(&tv, NULL);
+				time_diff = (tv.tv_sec - prev_tv.tv_sec);
+				time_diff <<= 10;
+				time_diff += (tv.tv_usec - prev_tv.tv_usec);
+				if ((1 << 9) < time_diff)
+					eye_candy_incr += 50;
+				else if (50 < eye_candy_incr)
+					eye_candy_incr -= 50;
+			}
+			eye_candy += eye_candy_incr;
+		}
 		if (get_sha1_hex(line, sha1))
 			die("expected sha1, got garbage:\n %s", line);
 		hash = 0;
@@ -537,6 +574,8 @@ int main(int argc, char **argv)
 		}
 		add_object_entry(sha1, hash);
 	}
+	if (progress)
+		fprintf(stderr, "Done counting %d objects.\n", nr_objects);
 	if (non_empty && !nr_objects)
 		return 0;
 
-- 
1.1.6.g69c5

Re: [PATCH] fetch-clone progress: finishing touches.

[Linus Torvalds]

On Sat, 11 Feb 2006, Junio C Hamano wrote:
> 
>    BTW, don't you mean 512 down there???
> 
>         -	msecs += (int)(tv.tv_usec - prev_tv.tv_usec) >> 10;
>         +	msecs += usec_to_binarymsec(tv.tv_usec - prev_tv.tv_usec);
>         +
>                 if (msecs > 500) {
>                         prev_tv = tv;

Well, it's just a random number, but if you like 512 better than 500, go 
wild ;)

		Linus

Re: [PATCH] fetch-clone progress: finishing touches.

[Linus Torvalds]

On Sat, 11 Feb 2006, Junio C Hamano wrote:
> 
>  * While we are doing eye-candy, this makes the silence after
>    "Generating pack..." part a bit more bearable.
> 
>    Likes, dislikes, too-much's?

Too little, actually.

Your change makes

	git clone ssh://...

be silent again, until the download actually starts. The "isatty(2)" thing 
in git-pack-objects won't trigger, because it's actually a socket, not a 
tty ;/

ssh will only set up a pty pair if it starts an interactive shell, not if 
you use the "ssh host cmd" form.

		Linus

Re: [PATCH] fetch-clone progress: finishing touches.

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> ssh will only set up a pty pair if it starts an interactive shell, not if 
> you use the "ssh host cmd" form.

True.  Or we _could_ use "ssh -t", but I've decided to make
progress the default.  If some script wants quiet behaviour they
can say 'pack-objects -q'.