threads / patch / 10571

patch, 3 partsyet more progress display stuff

Subject: [PATCH 0/3] yet more progress display stuff

## tl;dr

5 messages between Nov 1, 2007 and Nov 2, 2007. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Nicolas Pitre· Nov 1, 2007, 20:59 UTC · lore

Here's 3 additional patches. The first one revert one change already burried deep in the topic branch based on Shawn's concern about prune-packed progress display handling.

The second is a generic improvement to the throughput display.

The last is my reimplementation of total transfer byte count, replacing Shawn's proposed patch which suffered from a few issues.

Nicolas
Nicolas Pitre· Nov 1, 2007, 20:59 UTC · re: Nicolas Pitre · lore

[PATCH 1/3] return the prune-packed progress display to the inner loop

This reverts commit 0e549137966feb016927a827fb6e359aec8264a3 so to return to the same state as commit b5d72f0a4cd3cce945ca0d37e4fa0ebbfcdcdb52.

On Wed, 31 Oct 2007, Shawn O. Pearce wrote:
Show 5 quoted lines
> During my testing with a 40,000 loose object case (yea, I fully
> unpacked a git.git clone I had laying around) my system stalled
> hard in the first object directory.  A *lot* longer than 1 second.
> So I got no progress meter for a long time, and then a progress
> meter appeared on the second directory.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-prune-packed.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
Show changes to builtin-prune-packed.c +1 −2
diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index f4287da..23faf31 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -15,8 +15,6 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 	struct dirent *de;
 	char hex[40];
 
-	display_progress(progress, i + 1);
-
 	sprintf(hex, "%02x", i);
 	while ((de = readdir(dir)) != NULL) {
 		unsigned char sha1[20];
@@ -32,6 +30,7 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
 			printf("rm -f %s\n", pathname);
 		else if (unlink(pathname) < 0)
 			error("unable to unlink %s", pathname);
+		display_progress(progress, i + 1);
 	}
 	pathname[len] = 0;
 	rmdir(pathname);
-- 
1.5.3.4.279.gb2d9d-dirty
Nicolas Pitre· Nov 1, 2007, 20:59 UTC · re: Nicolas Pitre · lore

[PATCH 2/3] make sure throughput display gets updated even if progress doesn't move

Currently the progress/throughput display update happens only through display_progress(). If the progress based on object count remains unchanged because a large object is being received, the latest throughput won't be displayed. The display update should occur through display_throughput() as well.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 progress.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Show changes to progress.c +3 −0
diff --git a/progress.c b/progress.c
index 34a5961..39d5d2c 100644
--- a/progress.c
+++ b/progress.c
@@ -160,6 +160,9 @@ void display_throughput(struct progress *progress, unsigned long n)
 		tp->last_misecs[tp->idx] = misecs;
 		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
 		tp->count = 0;
+
+		if (progress->last_value != -1 && progress_update)
+			display(progress, progress->last_value, 0);
 	}
 }
 
-- 
1.5.3.4.279.gb2d9d-dirty
Nicolas Pitre· Nov 1, 2007, 20:59 UTC · re: Nicolas Pitre · lore

[PATCH 3/3] Show total transferred as part of throughput progress

Right now it is infeasible to offer to the user a reasonable concept of when a clone will be complete as we aren't able to come up with the final pack size until after we have actually transferred the entire thing to the client. However in many cases users can work with a rough rule-of-thumb; for example it is somewhat well known that git.git is about 16 MiB today and that linux-2.6.git is over 120 MiB.

We now show the total amount of data we have transferred over the network as part of the throughput meter, organizing it in "human friendly" terms like `ls -h` would do. Users can glance at this, see that the total transferred size is about 3 MiB, see the throughput of X KiB/sec, and determine a reasonable figure of about when the clone will be complete, assuming they know the rough size of the source repository or are able to obtain it.

This is also a helpful indicator that there is progress being made even if we stall on a very large object. The thoughput meter may remain relatively constant and the percentage complete and object count won't be changing, but the total transferred will be increasing as additional data is received for this object.

[from an initial proposal from Shawn O. Pearce]
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 progress.c |   29 ++++++++++++++++++++++++++---
 1 files changed, 26 insertions(+), 3 deletions(-)
Show changes to progress.c +26 −3
diff --git a/progress.c b/progress.c
index 39d5d2c..3f6a602 100644
--- a/progress.c
+++ b/progress.c
@@ -15,13 +15,14 @@
 
 struct throughput {
 	struct timeval prev_tv;
+	off_t total;
 	unsigned long count;
 	unsigned long avg_bytes;
 	unsigned long last_bytes[TP_IDX_MAX];
 	unsigned int avg_misecs;
 	unsigned int last_misecs[TP_IDX_MAX];
 	unsigned int idx;
-	char display[20];
+	char display[32];
 };
 
 struct progress {
@@ -128,6 +129,7 @@ void display_throughput(struct progress *progress, unsigned long n)
 		return;
 	}
 
+	tp->total += n;
 	tp->count += n;
 
 	/*
@@ -149,11 +151,32 @@ void display_throughput(struct progress *progress, unsigned long n)
 	misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
 
 	if (misecs > 512) {
+		int l = sizeof(tp->display);
 		tp->prev_tv = tv;
 		tp->avg_bytes += tp->count;
 		tp->avg_misecs += misecs;
-		snprintf(tp->display, sizeof(tp->display),
-			 ", %lu KiB/s", tp->avg_bytes / tp->avg_misecs);
+
+		if (tp->total > 1 << 30) {
+			l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
+				      (int)(tp->total >> 30),
+				      (int)(tp->total & ((1 << 30) - 1)) / 10737419);
+		} else if (tp->total > 1 << 20) {
+			l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
+				      (int)(tp->total >> 20),
+				      ((int)(tp->total & ((1 << 20) - 1))
+				       * 100) >> 20);
+		} else if (tp->total > 1 << 10) {
+			l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
+				      (int)(tp->total >> 10),
+				      ((int)(tp->total & ((1 << 10) - 1))
+				       * 100) >> 10);
+		} else {
+			l -= snprintf(tp->display, l, ", %u bytes",
+				      (int)tp->total);
+		}
+		snprintf(tp->display + sizeof(tp->display) - l, l,
+			 " | %lu KiB/s", tp->avg_bytes / tp->avg_misecs);
+
 		tp->avg_bytes -= tp->last_bytes[tp->idx];
 		tp->avg_misecs -= tp->last_misecs[tp->idx];
 		tp->last_bytes[tp->idx] = tp->count;
-- 
1.5.3.4.279.gb2d9d-dirty
Shawn O. Pearce· Nov 2, 2007, 04:59 UTC · re: Nicolas Pitre · lore

Re: [PATCH 3/3] Show total transferred as part of throughput progress

Nicolas Pitre <nico@cam.org> wrote:
Show 7 quoted lines
> Right now it is infeasible to offer to the user a reasonable concept
> of when a clone will be complete as we aren't able to come up with
> the final pack size until after we have actually transferred the
> entire thing to the client.  However in many cases users can work
> with a rough rule-of-thumb; for example it is somewhat well known
> that git.git is about 16 MiB today and that linux-2.6.git is over
> 120 MiB.
...
> [from an initial proposal from Shawn O. Pearce]
Thanks for rewriting this.  I agree, your replacement patch is
much better looking than my proposal.  I also see Junio has already
applied it to next.  Excellent.
 
-- 
Shawn.

← back to recent threads