# [PATCH] nicer eye candies for pack-objects 6 messages from 2006-02-22 to 2006-02-22. Participants: Nicolas Pitre, Junio C Hamano, Linus Torvalds, Andreas Ericsson. Thread: https://gitlist.dev/t/3439 ## Nicolas Pitre, 2006-02-22 21:00 Subject: [PATCH] nicer eye candies for pack-objects Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0602221549250.5606%40localhost.localdomain ``` 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 --- 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 +#include 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); ``` ## Junio C Hamano, 2006-02-22 22:27 Subject: Re: [PATCH] nicer eye candies for pack-objects Message-ID: <7vy803kp1n.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vy803kp1n.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` 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) ``` ## Nicolas Pitre, 2006-02-22 22:37 Subject: Re: [PATCH] nicer eye candies for pack-objects Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0602221733030.5606%40localhost.localdomain In-Reply-To: <7vy803kp1n.fsf@assigned-by-dhcp.cox.net> ``` 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 ``` ## Linus Torvalds, 2006-02-22 23:05 Subject: Re: [PATCH] nicer eye candies for pack-objects Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0602221502410.3771%40g5.osdl.org In-Reply-To: ``` 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 ``` ## Andreas Ericsson, 2006-02-22 23:19 Subject: Re: [PATCH] nicer eye candies for pack-objects Message-ID: <43FCF168.1060802@op5.se> URL: https://gitlist.dev/e/43FCF168.1060802%40op5.se In-Reply-To: ``` 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 ``` ## Nicolas Pitre, 2006-02-22 23:40 Subject: Re: [PATCH] nicer eye candies for pack-objects Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0602221835570.5606%40localhost.localdomain In-Reply-To: ``` 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 ```