{"thread":{"id":"3439","subject":"[PATCH] nicer eye candies for pack-objects","startedAt":"2006-02-22T21:00:08Z","lastAt":"2006-02-22T23:40:14Z","messageCount":6,"participants":["Nicolas Pitre","Junio C Hamano","Linus Torvalds","Andreas Ericsson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"16585","messageId":"Pine.LNX.4.64.0602221549250.5606@localhost.localdomain","threadId":"3439","inReplyTo":null,"subject":"[PATCH] nicer eye candies for pack-objects","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-02-22T21:00:08Z","receivedAt":"2006-02-22T21:00:08Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"This provides a stable and simpler progress reporting mechanism that \nupdates progress as often as possible but accurately not updating more \nthan once a second.  The deltification phase is also made more \ninteresting to watch (since repacking a big repository and only seeing a \ndot appear once every many seconds is rather boring and doesn't provide \nmuch food for anticipation).\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n\n---\n\ndiff --git a/pack-objects.c b/pack-objects.c\nindex 0c9f4c9..7c89dc7 100644\n--- a/pack-objects.c\n+++ b/pack-objects.c\n@@ -4,6 +4,7 @@\n #include \"pack.h\"\n #include \"csum-file.h\"\n #include <sys/time.h>\n+#include <signal.h>\n \n 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\";\n \n@@ -661,17 +661,22 @@ static int try_delta(struct unpacked *cu\n \treturn 0;\n }\n \n+static volatile int progress_update = 0;\n+static void progress_interval(int signum)\n+{\n+\tsignal(SIGALRM, progress_interval);\n+\tprogress_update = 1;\n+}\n+\n static void find_deltas(struct object_entry **list, int window, int depth)\n {\n \tint i, idx;\n \tunsigned int array_size = window * sizeof(struct unpacked);\n \tstruct unpacked *array = xmalloc(array_size);\n-\tint eye_candy;\n \n \tmemset(array, 0, array_size);\n \ti = nr_objects;\n \tidx = 0;\n-\teye_candy = i - (nr_objects / 20);\n \n \twhile (--i >= 0) {\n \t\tstruct object_entry *entry = list[i];\n@@ -680,9 +685,10 @@ static void find_deltas(struct object_en\n \t\tchar type[10];\n \t\tint j;\n \n-\t\tif (progress && i <= eye_candy) {\n-\t\t\teye_candy -= nr_objects / 20;\n-\t\t\tfputc('.', stderr);\n+\t\tif (progress_update || i == 0) {\n+\t\t\tfprintf(stderr, \"Deltifying (%d %d%%)\\r\",\n+\t\t\t\tnr_objects-i, (nr_objects-i) * 100/nr_objects);\n+\t\t\tprogress_update = 0;\n \t\t}\n \n \t\tif (entry->delta)\n@@ -714,6 +720,9 @@ static void find_deltas(struct object_en\n \t\t\tidx = 0;\n \t}\n \n+\tif (progress)\n+\t\tfputc('\\n', stderr);\n+\n \tfor (i = 0; i < window; ++i)\n \t\tfree(array[i].data);\n \tfree(array);\n@@ -721,17 +730,10 @@ static void find_deltas(struct object_en\n \n static void prepare_pack(int window, int depth)\n {\n-\tif (progress)\n-\t\tfprintf(stderr, \"Packing %d objects\", nr_objects);\n \tget_object_details();\n-\tif (progress)\n-\t\tfputc('.', stderr);\n-\n \tsorted_by_type = create_sorted_list(type_size_sort);\n \tif (window && depth)\n \t\tfind_deltas(sorted_by_type, window+1, depth);\n-\tif (progress)\n-\t\tfputc('\\n', stderr);\n \twrite_pack_file();\n }\n \n@@ -796,10 +798,6 @@ int main(int argc, char **argv)\n \tint window = 10, depth = 10, pack_to_stdout = 0;\n \tstruct object_entry **list;\n \tint i;\n-\tstruct timeval prev_tv;\n-\tint eye_candy = 0;\n-\tint eye_candy_incr = 500;\n-\n \n \tsetup_git_directory();\n \n@@ -856,30 +854,25 @@ int main(int argc, char **argv)\n \t\tusage(pack_usage);\n \n \tprepare_packed_git();\n+\n \tif (progress) {\n+\t\tstruct itimerval v;\n+\t\tv.it_interval.tv_sec = 1;\n+\t\tv.it_interval.tv_usec = 0;\n+\t\tv.it_value = v.it_interval;\n+\t\tsignal(SIGALRM, progress_interval);\n+\t\tsetitimer(ITIMER_REAL, &v, NULL);\n \t\tfprintf(stderr, \"Generating pack...\\n\");\n-\t\tgettimeofday(&prev_tv, NULL);\n \t}\n+\n \twhile (fgets(line, sizeof(line), stdin) != NULL) {\n \t\tunsigned int hash;\n \t\tchar *p;\n \t\tunsigned char sha1[20];\n \n-\t\tif (progress && (eye_candy <= nr_objects)) {\n+\t\tif (progress_update) {\n \t\t\tfprintf(stderr, \"Counting objects...%d\\r\", nr_objects);\n-\t\t\tif (eye_candy && (50 <= eye_candy_incr)) {\n-\t\t\t\tstruct timeval tv;\n-\t\t\t\tint time_diff;\n-\t\t\t\tgettimeofday(&tv, NULL);\n-\t\t\t\ttime_diff = (tv.tv_sec - prev_tv.tv_sec);\n-\t\t\t\ttime_diff <<= 10;\n-\t\t\t\ttime_diff += (tv.tv_usec - prev_tv.tv_usec);\n-\t\t\t\tif ((1 << 9) < time_diff)\n-\t\t\t\t\teye_candy_incr += 50;\n-\t\t\t\telse if (50 < eye_candy_incr)\n-\t\t\t\t\teye_candy_incr -= 50;\n-\t\t\t}\n-\t\t\teye_candy += eye_candy_incr;\n+\t\t\tprogress_update = 0;\n \t\t}\n \t\tif (get_sha1_hex(line, sha1))\n \t\t\tdie(\"expected sha1, got garbage:\\n %s\", line);\n"},{"id":"16590","messageId":"7vy803kp1n.fsf@assigned-by-dhcp.cox.net","threadId":"3439","inReplyTo":"Pine.LNX.4.64.0602221549250.5606@localhost.localdomain","subject":"Re: [PATCH] nicer eye candies for pack-objects","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-22T22:27:00Z","receivedAt":"2006-02-22T22:27:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I like this, but like the \"every second or every percent\nwhichever comes first\" unpack-objects does even better.  How\nabout something like this on top of your patch?\n\n---\ndiff --git a/pack-objects.c b/pack-objects.c\nindex 5e1e14c..d05ab23 100644\n--- a/pack-objects.c\n+++ b/pack-objects.c\n@@ -674,10 +674,14 @@ static void find_deltas(struct object_en\n \tint i, idx;\n \tunsigned int array_size = window * sizeof(struct unpacked);\n \tstruct unpacked *array = xmalloc(array_size);\n+\tunsigned processed = 0;\n+\tunsigned last_percent = 999;\n \n \tmemset(array, 0, array_size);\n \ti = nr_objects;\n \tidx = 0;\n+\tif (progress)\n+\t\tfprintf(stderr, \"Deltifying %d objects.\\n\", nr_objects);\n \n \twhile (--i >= 0) {\n \t\tstruct object_entry *entry = list[i];\n@@ -686,10 +690,15 @@ static void find_deltas(struct object_en\n \t\tchar type[10];\n \t\tint j;\n \n-\t\tif (progress_update || i == 0) {\n-\t\t\tfprintf(stderr, \"Deltifying (%d %d%%)\\r\",\n-\t\t\t\tnr_objects-i, (nr_objects-i) * 100/nr_objects);\n-\t\t\tprogress_update = 0;\n+\t\tprocessed++;\n+\t\tif (progress) {\n+\t\t\tunsigned percent = processed * 100 / nr_objects;\n+\t\t\tif (percent != last_percent || progress_update) {\n+\t\t\t\tfprintf(stderr, \"%4u%% (%u/%u) done\\r\",\n+\t\t\t\t\tpercent, processed, nr_objects);\n+\t\t\t\tprogress_update = 0;\n+\t\t\t\tlast_percent = percent;\n+\t\t\t}\n \t\t}\n \n \t\tif (entry->delta)\n"},{"id":"16591","messageId":"Pine.LNX.4.64.0602221733030.5606@localhost.localdomain","threadId":"3439","inReplyTo":"7vy803kp1n.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] nicer eye candies for pack-objects","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-02-22T22:37:06Z","receivedAt":"2006-02-22T22:37:06Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 22 Feb 2006, Junio C Hamano wrote:\n\n> I like this, but like the \"every second or every percent\n> whichever comes first\" unpack-objects does even better.  How\n> about something like this on top of your patch?\n\nWell... my concern is (if I'm right) that this status is generated \nremotely and sent over the network when performing a fetch.  The \"every \npercent\" might in this case generate quite some significant overhead if \nthe pack is small.\n\nAlso (personal opinion) such progress numbers are harder to read when \nthey change too fast.\n\n\nNicolas\n"},{"id":"16593","messageId":"Pine.LNX.4.64.0602221502410.3771@g5.osdl.org","threadId":"3439","inReplyTo":"Pine.LNX.4.64.0602221733030.5606@localhost.localdomain","subject":"Re: [PATCH] nicer eye candies for pack-objects","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-02-22T23:05:46Z","receivedAt":"2006-02-22T23:05:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 22 Feb 2006, Nicolas Pitre wrote:\n\n> On Wed, 22 Feb 2006, Junio C Hamano wrote:\n> \n> > I like this, but like the \"every second or every percent\n> > whichever comes first\" unpack-objects does even better.  How\n> > about something like this on top of your patch?\n> \n> Well... my concern is (if I'm right) that this status is generated \n> remotely and sent over the network when performing a fetch.  The \"every \n> percent\" might in this case generate quite some significant overhead if \n> the pack is small.\n\nWell, my thinking behind the original unpack-objects behaviour was that we \ndon't really care about the max 100 extra packets. \n\nThe days of 150 baud line printers are gone. I cannot imagine any valid \nsituation where you can't send a hundred small packets to update the \nscreen.. And it did make a nice visual difference.\n\n(In fact, over a network, if the line really is slow, you'll find that \nNagle will fix things, and you'll just see one extra - but obviously \nslightly bigger - packet).\n\n\t\tLinus\n"},{"id":"16594","messageId":"43FCF168.1060802@op5.se","threadId":"3439","inReplyTo":"Pine.LNX.4.64.0602221733030.5606@localhost.localdomain","subject":"Re: [PATCH] nicer eye candies for pack-objects","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-02-22T23:19:04Z","receivedAt":"2006-02-22T23:19:04Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Nicolas Pitre wrote:\n> On Wed, 22 Feb 2006, Junio C Hamano wrote:\n> \n> \n>>I like this, but like the \"every second or every percent\n>>whichever comes first\" unpack-objects does even better.  How\n>>about something like this on top of your patch?\n> \n> \n> Well... my concern is (if I'm right) that this status is generated \n> remotely and sent over the network when performing a fetch.  The \"every \n> percent\" might in this case generate quite some significant overhead if \n> the pack is small.\n> \n\nBut if the pack is small it won't matter. It's when it's big we want to \nknow about it (and then the each-percent method is better, really).\n\n> Also (personal opinion) such progress numbers are harder to read when \n> they change too fast.\n> \n\nI don't know about the rest of the world, but when I see numbers \ncounting up with a percent-sign behind them I do some mental math to see \nhow many brain-ticks go between each increment and then multiply with \n100 to see if I need to get a beer while waiting. I don't really care \nwhat number it shows if it flashes too fast to read.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"16595","messageId":"Pine.LNX.4.64.0602221835570.5606@localhost.localdomain","threadId":"3439","inReplyTo":"Pine.LNX.4.64.0602221502410.3771@g5.osdl.org","subject":"Re: [PATCH] nicer eye candies for pack-objects","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-02-22T23:40:14Z","receivedAt":"2006-02-22T23:40:14Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 22 Feb 2006, Linus Torvalds wrote:\n\n> Well, my thinking behind the original unpack-objects behaviour was that we \n> don't really care about the max 100 extra packets. \n\nObviously, the \"every percent\" has at most 100 additional packets.\n\nAnd if it updates too fast to be readable, that means it'll be over in a \nsnap anyway.\n\nSo I don't have any objections left.\n\n\nNicolas\n"}]}