[PATCH] nicer eye candies for pack-objects

[PATCH] nicer eye candies for pack-objects

[Nicolas Pitre]

This provides a stable and simpler progress reporting mechanism that 
updates progress as often as possible but accurately not updating more 
than once a second.  The deltification phase is also made more 
interesting to watch (since repacking a big repository and only seeing a 
dot appear once every many seconds is rather boring and doesn't provide 
much food for anticipation).

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

diff --git a/pack-objects.c b/pack-objects.c
index 0c9f4c9..7c89dc7 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -4,6 +4,7 @@
 #include "pack.h"
 #include "csum-file.h"
 #include <sys/time.h>
+#include <signal.h>
 
 static const char pack_usage[] = "git-pack-objects [-q] [--no-reuse-delta] [--non-empty] [--local] [--incremental] [--window=N] [--depth=N] {--stdout | base-name} < object-list";
 
@@ -661,17 +661,22 @@ static int try_delta(struct unpacked *cu
 	return 0;
 }
 
+static volatile int progress_update = 0;
+static void progress_interval(int signum)
+{
+	signal(SIGALRM, progress_interval);
+	progress_update = 1;
+}
+
 static void find_deltas(struct object_entry **list, int window, int depth)
 {
 	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];
@@ -680,9 +685,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);
+		if (progress_update || i == 0) {
+			fprintf(stderr, "Deltifying (%d %d%%)\r",
+				nr_objects-i, (nr_objects-i) * 100/nr_objects);
+			progress_update = 0;
 		}
 
 		if (entry->delta)
@@ -714,6 +720,9 @@ static void find_deltas(struct object_en
 			idx = 0;
 	}
 
+	if (progress)
+		fputc('\n', stderr);
+
 	for (i = 0; i < window; ++i)
 		free(array[i].data);
 	free(array);
@@ -721,17 +730,10 @@ static void find_deltas(struct object_en
 
 static void prepare_pack(int window, int depth)
 {
-	if (progress)
-		fprintf(stderr, "Packing %d objects", nr_objects);
 	get_object_details();
-	if (progress)
-		fputc('.', stderr);
-
 	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();
 }
 
@@ -796,10 +798,6 @@ 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();
 
@@ -856,30 +854,25 @@ int main(int argc, char **argv)
 		usage(pack_usage);
 
 	prepare_packed_git();
+
 	if (progress) {
+		struct itimerval v;
+		v.it_interval.tv_sec = 1;
+		v.it_interval.tv_usec = 0;
+		v.it_value = v.it_interval;
+		signal(SIGALRM, progress_interval);
+		setitimer(ITIMER_REAL, &v, NULL);
 		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)) {
+		if (progress_update) {
 			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;
+			progress_update = 0;
 		}
 		if (get_sha1_hex(line, sha1))
 			die("expected sha1, got garbage:\n %s", line);

Re: [PATCH] nicer eye candies for pack-objects

[Junio C Hamano]

I like this, but like the "every second or every percent
whichever comes first" unpack-objects does even better.  How
about something like this on top of your patch?

---
diff --git a/pack-objects.c b/pack-objects.c
index 5e1e14c..d05ab23 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -674,10 +674,14 @@ static void find_deltas(struct object_en
 	int i, idx;
 	unsigned int array_size = window * sizeof(struct unpacked);
 	struct unpacked *array = xmalloc(array_size);
+	unsigned processed = 0;
+	unsigned last_percent = 999;
 
 	memset(array, 0, array_size);
 	i = nr_objects;
 	idx = 0;
+	if (progress)
+		fprintf(stderr, "Deltifying %d objects.\n", nr_objects);
 
 	while (--i >= 0) {
 		struct object_entry *entry = list[i];
@@ -686,10 +690,15 @@ static void find_deltas(struct object_en
 		char type[10];
 		int j;
 
-		if (progress_update || i == 0) {
-			fprintf(stderr, "Deltifying (%d %d%%)\r",
-				nr_objects-i, (nr_objects-i) * 100/nr_objects);
-			progress_update = 0;
+		processed++;
+		if (progress) {
+			unsigned percent = processed * 100 / nr_objects;
+			if (percent != last_percent || progress_update) {
+				fprintf(stderr, "%4u%% (%u/%u) done\r",
+					percent, processed, nr_objects);
+				progress_update = 0;
+				last_percent = percent;
+			}
 		}
 
 		if (entry->delta)

Re: [PATCH] nicer eye candies for pack-objects

[Nicolas Pitre]

On Wed, 22 Feb 2006, Junio C Hamano wrote:

> I like this, but like the "every second or every percent
> whichever comes first" unpack-objects does even better.  How
> about something like this on top of your patch?

Well... my concern is (if I'm right) that this status is generated 
remotely and sent over the network when performing a fetch.  The "every 
percent" might in this case generate quite some significant overhead if 
the pack is small.

Also (personal opinion) such progress numbers are harder to read when 
they change too fast.


Nicolas

Re: [PATCH] nicer eye candies for pack-objects

[Linus Torvalds]

On Wed, 22 Feb 2006, Nicolas Pitre wrote:

> On Wed, 22 Feb 2006, Junio C Hamano wrote:
> 
> > I like this, but like the "every second or every percent
> > whichever comes first" unpack-objects does even better.  How
> > about something like this on top of your patch?
> 
> Well... my concern is (if I'm right) that this status is generated 
> remotely and sent over the network when performing a fetch.  The "every 
> percent" might in this case generate quite some significant overhead if 
> the pack is small.

Well, my thinking behind the original unpack-objects behaviour was that we 
don't really care about the max 100 extra packets. 

The days of 150 baud line printers are gone. I cannot imagine any valid 
situation where you can't send a hundred small packets to update the 
screen.. And it did make a nice visual difference.

(In fact, over a network, if the line really is slow, you'll find that 
Nagle will fix things, and you'll just see one extra - but obviously 
slightly bigger - packet).

		Linus

Re: [PATCH] nicer eye candies for pack-objects

[Andreas Ericsson]

Nicolas Pitre wrote:
> On Wed, 22 Feb 2006, Junio C Hamano wrote:
> 
> 
>>I like this, but like the "every second or every percent
>>whichever comes first" unpack-objects does even better.  How
>>about something like this on top of your patch?
> 
> 
> Well... my concern is (if I'm right) that this status is generated 
> remotely and sent over the network when performing a fetch.  The "every 
> percent" might in this case generate quite some significant overhead if 
> the pack is small.
> 

But if the pack is small it won't matter. It's when it's big we want to 
know about it (and then the each-percent method is better, really).

> Also (personal opinion) such progress numbers are harder to read when 
> they change too fast.
> 

I don't know about the rest of the world, but when I see numbers 
counting up with a percent-sign behind them I do some mental math to see 
how many brain-ticks go between each increment and then multiply with 
100 to see if I need to get a beer while waiting. I don't really care 
what number it shows if it flashes too fast to read.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH] nicer eye candies for pack-objects

[Nicolas Pitre]

On Wed, 22 Feb 2006, Linus Torvalds wrote:

> Well, my thinking behind the original unpack-objects behaviour was that we 
> don't really care about the max 100 extra packets. 

Obviously, the "every percent" has at most 100 additional packets.

And if it updates too fast to be readable, that means it'll be over in a 
snap anyway.

So I don't have any objections left.


Nicolas