{"thread":{"id":"18715","subject":"[PATCH 1/2] git-repack: use non-dashed update-server-info","startedAt":"2009-04-04T16:59:55Z","lastAt":"2009-04-11T19:42:08Z","messageCount":14,"participants":["Dan McGee","Jeff King","Nicolas Pitre","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"110335","messageId":"1238864396-8964-1-git-send-email-dpmcgee@gmail.com","threadId":"18715","inReplyTo":null,"subject":"[PATCH 1/2] git-repack: use non-dashed update-server-info","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2009-04-04T16:59:55Z","receivedAt":"2009-04-04T16:59:55Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"Signed-off-by: Dan McGee \n---\n git-repack.sh | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 1782a23..0868734 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -181,5 +181,5 @@ fi\n \n case \"$no_update_info\" in\n t) : ;;\n-*) git-update-server-info ;;\n+*) git update-server-info ;;\n esac\n-- \n1.6.2.2.404.ge96f3.dirty\n"},{"id":"110336","messageId":"1238864396-8964-2-git-send-email-dpmcgee@gmail.com","threadId":"18715","inReplyTo":"1238864396-8964-1-git-send-email-dpmcgee@gmail.com","subject":"[PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2009-04-04T16:59:56Z","receivedAt":"2009-04-04T16:59:56Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"In the case of a small repository, pack-objects is smart enough to not start\nmore threads than necessary. However, the output to the user always reports\nthe value of the pack.threads configuration and not the real number of\nthreads to be used. This is easily fixed by moving the printing of the\nmessage after we have partitioned our work.\n\n(pack.threads is on autodetect and would be set to 4)\n$ git-repack -a -d -f\nCounting objects: 55, done.\nDelta compression using 2 threads.\nCompressing objects: 100% (48/48), done.\nWriting objects: 100% (55/55), done.\nTotal 55 (delta 10), reused 0 (delta 0)\n\nSigned-off-by: Dan McGee \n---\n builtin-pack-objects.c | 10 +++++++---\n 1 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 9fc3b35..8c1b036 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1611,9 +1611,6 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\tfind_deltas(list, &list_size, window, depth, processed);\n \t\treturn;\n \t}\n-\tif (progress > pack_to_stdout)\n-\t\tfprintf(stderr, \"Delta compression using %d threads.\\n\",\n-\t\t\t\tdelta_search_threads);\n \n \t/* Partition the work amongst work threads. */\n \tfor (i = 0; i < delta_search_threads; i++) {\n@@ -1638,11 +1635,18 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\tp[i].list = list;\n \t\tp[i].list_size = sub_size;\n \t\tp[i].remaining = sub_size;\n+\t\tif(sub_size)\n+\t\t\tactive_threads++;\n \n \t\tlist += sub_size;\n \t\tlist_size -= sub_size;\n \t}\n \n+\tif (progress > pack_to_stdout)\n+\t\tfprintf(stderr, \"Delta compression using %d threads.\\n\",\n+\t\t\t\tactive_threads);\n+\tactive_threads = 0;\n+\n \t/* Start work threads. */\n \tfor (i = 0; i < delta_search_threads; i++) {\n \t\tif (!p[i].list_size)\n-- \n1.6.2.2.404.ge96f3.dirty\n"},{"id":"110339","messageId":"20090404180601.GA14888@coredump.intra.peff.net","threadId":"18715","inReplyTo":"1238864396-8964-2-git-send-email-dpmcgee@gmail.com","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-04T18:06:01Z","receivedAt":"2009-04-04T18:06:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 04, 2009 at 11:59:56AM -0500, Dan McGee wrote:\n\n> In the case of a small repository, pack-objects is smart enough to not start\n> more threads than necessary. However, the output to the user always reports\n> the value of the pack.threads configuration and not the real number of\n> threads to be used. This is easily fixed by moving the printing of the\n> message after we have partitioned our work.\n> \n> (pack.threads is on autodetect and would be set to 4)\n> $ git-repack -a -d -f\n> Counting objects: 55, done.\n> Delta compression using 2 threads.\n> Compressing objects: 100% (48/48), done.\n> Writing objects: 100% (55/55), done.\n> Total 55 (delta 10), reused 0 (delta 0)\n\nThat makes sense to me, though I wonder if it may confuse and frustrate\nusers who are expecting their awesome quad-core machine to be using 4\nthreads when it only uses 2. Is it worth printing both values, or some\nindicator that we could have been using more?\n\n-Peff\n"},{"id":"110340","messageId":"449c10960904041120j38086192s25070912b0371c09@mail.gmail.com","threadId":"18715","inReplyTo":"20090404180601.GA14888@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2009-04-04T18:20:18Z","receivedAt":"2009-04-04T18:20:18Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"On Sat, Apr 4, 2009 at 1:06 PM, Jeff King wrote:\n> On Sat, Apr 04, 2009 at 11:59:56AM -0500, Dan McGee wrote:\n>\n>> In the case of a small repository, pack-objects is smart enough to not start\n>> more threads than necessary. However, the output to the user always reports\n>> the value of the pack.threads configuration and not the real number of\n>> threads to be used. This is easily fixed by moving the printing of the\n>> message after we have partitioned our work.\n>>\n>> (pack.threads is on autodetect and would be set to 4)\n>> $ git-repack -a -d -f\n>> Counting objects: 55, done.\n>> Delta compression using 2 threads.\n>> Compressing objects: 100% (48/48), done.\n>> Writing objects: 100% (55/55), done.\n>> Total 55 (delta 10), reused 0 (delta 0)\n>\n> That makes sense to me, though I wonder if it may confuse and frustrate\n> users who are expecting their awesome quad-core machine to be using 4\n> threads when it only uses 2. Is it worth printing both values, or some\n> indicator that we could have been using more?\n\nI thought of this, but decided it wasn't really worth it. The default\nwindow size of 10 makes it a very rare case that you will use fewer\nthan 4 threads. With the default, each thread needs a minimum of 20\nobjects, so even a 100-object repository would spawn the 4 threads.\n\nI wouldn't be opposed to printing something special when\nactive_threads != delta_search_threads if other people do think it to\nbe necessary though.\n\n-Dan\n"},{"id":"110351","messageId":"20090404232505.GA26906@coredump.intra.peff.net","threadId":"18715","inReplyTo":"449c10960904041120j38086192s25070912b0371c09@mail.gmail.com","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-04T23:25:05Z","receivedAt":"2009-04-04T23:25:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 04, 2009 at 01:20:18PM -0500, Dan McGee wrote:\n\n> > That makes sense to me, though I wonder if it may confuse and frustrate\n> > users who are expecting their awesome quad-core machine to be using 4\n> > threads when it only uses 2. Is it worth printing both values, or some\n> > indicator that we could have been using more?\n> \n> I thought of this, but decided it wasn't really worth it. The default\n> window size of 10 makes it a very rare case that you will use fewer\n> than 4 threads. With the default, each thread needs a minimum of 20\n> objects, so even a 100-object repository would spawn the 4 threads.\n\nGood point. Though by that logic, isn't your patch also not worth it\n(i.e., it is unlikely not to fill the threads, so the output will be the\nsame with or without it)?\n\nI still think yours is an improvement, though, however slight.\n\n-Peff\n"},{"id":"110359","messageId":"alpine.LFD.2.00.0904042001540.6741@xanadu.home","threadId":"18715","inReplyTo":"20090404232505.GA26906@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-04-05T00:11:38Z","receivedAt":"2009-04-05T00:11:38Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sat, 4 Apr 2009, Jeff King wrote:\n\n> On Sat, Apr 04, 2009 at 01:20:18PM -0500, Dan McGee wrote:\n> \n> > > That makes sense to me, though I wonder if it may confuse and frustrate\n> > > users who are expecting their awesome quad-core machine to be using 4\n> > > threads when it only uses 2. Is it worth printing both values, or some\n> > > indicator that we could have been using more?\n> > \n> > I thought of this, but decided it wasn't really worth it. The default\n> > window size of 10 makes it a very rare case that you will use fewer\n> > than 4 threads. With the default, each thread needs a minimum of 20\n> > objects, so even a 100-object repository would spawn the 4 threads.\n> \n> Good point. Though by that logic, isn't your patch also not worth it\n> (i.e., it is unlikely not to fill the threads, so the output will be the\n> same with or without it)?\n> \n> I still think yours is an improvement, though, however slight.\n\nI don't think this is worth it at all.\n\nThis display is there mainly to confirm expected number of available \nthreads. The number of actually active threads is an implementation \ndetail. Sure if the number of objects is too low, or if the window size \nis too large, then the number of active threads will be lower. But in \npractice it is also possible that with some patological object set you \nend up with 2 threads out of 4 completing very quickly and the other 2 \nthreads still busy with big objects and total remaining work set too \nsmall to split it further amongst idle threads, meaning that you'll end \nup with only 2 busy CPUs even though the display said 4 threads \ninitially even with this patch.\n\nIn other words I don't think this patch is a good idea as we don't \nupdate the display with remaining active threads along the way.\n\n\nNicolas\n"},{"id":"110499","messageId":"449c10960904051909v5ec5d7danc10d13d9a1d613f0@mail.gmail.com","threadId":"18715","inReplyTo":"alpine.LFD.2.00.0904042001540.6741@xanadu.home","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2009-04-06T02:09:29Z","receivedAt":"2009-04-06T02:09:29Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"On Sat, Apr 4, 2009 at 7:11 PM, Nicolas Pitre wrote:\n> On Sat, 4 Apr 2009, Jeff King wrote:\n>\n>> On Sat, Apr 04, 2009 at 01:20:18PM -0500, Dan McGee wrote:\n>>\n>> > > That makes sense to me, though I wonder if it may confuse and frustrate\n>> > > users who are expecting their awesome quad-core machine to be using 4\n>> > > threads when it only uses 2. Is it worth printing both values, or some\n>> > > indicator that we could have been using more?\n>> >\n>> > I thought of this, but decided it wasn't really worth it. The default\n>> > window size of 10 makes it a very rare case that you will use fewer\n>> > than 4 threads. With the default, each thread needs a minimum of 20\n>> > objects, so even a 100-object repository would spawn the 4 threads.\n>>\n>> Good point. Though by that logic, isn't your patch also not worth it\n>> (i.e., it is unlikely not to fill the threads, so the output will be the\n>> same with or without it)?\n>>\n>> I still think yours is an improvement, though, however slight.\n>\n> I don't think this is worth it at all.\n>\n> This display is there mainly to confirm expected number of available\n> threads.  The number of actually active threads is an implementation\n> detail.  Sure if the number of objects is too low, or if the window size\n> is too large, then the number of active threads will be lower.  But in\n> practice it is also possible that with some patological object set you\n> end up with 2 threads out of 4 completing very quickly and the other 2\n> threads still busy with big objects and total remaining work set too\n> small to split it further amongst idle threads, meaning that you'll end\n> up with only 2 busy CPUs even though the display said 4 threads\n> initially even with this patch.\n>\n> In other words I don't think this patch is a good idea as we don't\n> update the display with remaining active threads along the way.\n\nWhy do we show this misleading at best piece of information at all\nthen? I'd rather completely remove it than show lies to the user. It\nsounds like it is only there for debugging purposes.\n\nIf we choose to keep it, I propose either accepting my patch so we are\nnot mislead, or dropping the thread count completely from the output\nand saying only something like \"Using multi-threaded delta\ncompression.\"\n\n-Dan\n"},{"id":"110501","messageId":"alpine.LFD.2.00.0904052220060.6741@xanadu.home","threadId":"18715","inReplyTo":"449c10960904051909v5ec5d7danc10d13d9a1d613f0@mail.gmail.com","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-04-06T02:31:04Z","receivedAt":"2009-04-06T02:31:04Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 5 Apr 2009, Dan McGee wrote:\n\n> On Sat, Apr 4, 2009 at 7:11 PM, Nicolas Pitre wrote:\n> > On Sat, 4 Apr 2009, Jeff King wrote:\n> >\n> >> On Sat, Apr 04, 2009 at 01:20:18PM -0500, Dan McGee wrote:\n> >>\n> >> > > That makes sense to me, though I wonder if it may confuse and frustrate\n> >> > > users who are expecting their awesome quad-core machine to be using 4\n> >> > > threads when it only uses 2. Is it worth printing both values, or some\n> >> > > indicator that we could have been using more?\n> >> >\n> >> > I thought of this, but decided it wasn't really worth it. The default\n> >> > window size of 10 makes it a very rare case that you will use fewer\n> >> > than 4 threads. With the default, each thread needs a minimum of 20\n> >> > objects, so even a 100-object repository would spawn the 4 threads.\n> >>\n> >> Good point. Though by that logic, isn't your patch also not worth it\n> >> (i.e., it is unlikely not to fill the threads, so the output will be the\n> >> same with or without it)?\n> >>\n> >> I still think yours is an improvement, though, however slight.\n> >\n> > I don't think this is worth it at all.\n> >\n> > This display is there mainly to confirm expected number of available\n> > threads.  The number of actually active threads is an implementation\n> > detail.  Sure if the number of objects is too low, or if the window size\n> > is too large, then the number of active threads will be lower.  But in\n> > practice it is also possible that with some patological object set you\n> > end up with 2 threads out of 4 completing very quickly and the other 2\n> > threads still busy with big objects and total remaining work set too\n> > small to split it further amongst idle threads, meaning that you'll end\n> > up with only 2 busy CPUs even though the display said 4 threads\n> > initially even with this patch.\n> >\n> > In other words I don't think this patch is a good idea as we don't\n> > update the display with remaining active threads along the way.\n> \n> Why do we show this misleading at best piece of information at all\n> then? I'd rather completely remove it than show lies to the user.\n\nAs you might imagine, I don't share your above appreciation.\n\n> It\n> sounds like it is only there for debugging purposes.\n\n... which is still worthwhile nevertheless.\n\n> If we choose to keep it, I propose either accepting my patch so we are\n> not mislead, or dropping the thread count completely from the output\n> and saying only something like \"Using multi-threaded delta\n> compression.\"\n\nYour patch is not better. Instead, it will confuse people who \nexplicitly told git to use x threads but the display might say x-y \nthreads, with 0 <= y < x.\n\nThe number currently displayed has real meaning: this is the number of \nthreads git is allowed to use. The number of threads it will actually \nuse is variable and it changes with time.\n\n\nNicolas\n"},{"id":"110502","messageId":"449c10960904051934l54d2d504w1af867dc53ef7dd7@mail.gmail.com","threadId":"18715","inReplyTo":"alpine.LFD.2.00.0904052220060.6741@xanadu.home","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2009-04-06T02:34:34Z","receivedAt":"2009-04-06T02:34:34Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"On Sun, Apr 5, 2009 at 9:31 PM, Nicolas Pitre wrote:\n> On Sun, 5 Apr 2009, Dan McGee wrote:\n>\n>> On Sat, Apr 4, 2009 at 7:11 PM, Nicolas Pitre wrote:\n>> > On Sat, 4 Apr 2009, Jeff King wrote:\n>> >\n>> >> On Sat, Apr 04, 2009 at 01:20:18PM -0500, Dan McGee wrote:\n>> >>\n>> >> > > That makes sense to me, though I wonder if it may confuse and frustrate\n>> >> > > users who are expecting their awesome quad-core machine to be using 4\n>> >> > > threads when it only uses 2. Is it worth printing both values, or some\n>> >> > > indicator that we could have been using more?\n>> >> >\n>> >> > I thought of this, but decided it wasn't really worth it. The default\n>> >> > window size of 10 makes it a very rare case that you will use fewer\n>> >> > than 4 threads. With the default, each thread needs a minimum of 20\n>> >> > objects, so even a 100-object repository would spawn the 4 threads.\n>> >>\n>> >> Good point. Though by that logic, isn't your patch also not worth it\n>> >> (i.e., it is unlikely not to fill the threads, so the output will be the\n>> >> same with or without it)?\n>> >>\n>> >> I still think yours is an improvement, though, however slight.\n>> >\n>> > I don't think this is worth it at all.\n>> >\n>> > This display is there mainly to confirm expected number of available\n>> > threads.  The number of actually active threads is an implementation\n>> > detail.  Sure if the number of objects is too low, or if the window size\n>> > is too large, then the number of active threads will be lower.  But in\n>> > practice it is also possible that with some patological object set you\n>> > end up with 2 threads out of 4 completing very quickly and the other 2\n>> > threads still busy with big objects and total remaining work set too\n>> > small to split it further amongst idle threads, meaning that you'll end\n>> > up with only 2 busy CPUs even though the display said 4 threads\n>> > initially even with this patch.\n>> >\n>> > In other words I don't think this patch is a good idea as we don't\n>> > update the display with remaining active threads along the way.\n>>\n>> Why do we show this misleading at best piece of information at all\n>> then? I'd rather completely remove it than show lies to the user.\n>\n> As you might imagine, I don't share your above appreciation.\n>\n>> It\n>> sounds like it is only there for debugging purposes.\n>\n> ... which is still worthwhile nevertheless.\n>\n>> If we choose to keep it, I propose either accepting my patch so we are\n>> not mislead, or dropping the thread count completely from the output\n>> and saying only something like \"Using multi-threaded delta\n>> compression.\"\n>\n> Your patch is not better.  Instead, it will confuse people who\n> explicitly told git to use x threads but the display might say x-y\n> threads, with 0 <= y < x.\n>\n> The number currently displayed has real meaning: this is the number of\n> threads git is allowed to use.  The number of threads it will actually\n> use is variable and it changes with time.\n\nWould something like this be more ideal then? I wouldn't be so\npersistent here if the current text wasn't misleading in a case like\nthe following:\n\ndmcgee@galway ~/projects/devtools (master)\n$ git push origin\nCounting objects: 13, done.\nDelta compression using 4 threads.\nCompressing objects: 100% (8/8), done.\nWriting objects: 100% (8/8), 1.28 KiB, done.\nTotal 8 (delta 6), reused 0 (delta 0)\nTo archlinux.org:/srv/projects/git/devtools.git\n bcb0e39..ea73c2b master -> master\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 9fc3b35..99181fd 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1612,7 +1612,7 @@ static void ll_find_deltas(struct object_entry\n**list, unsigned list_size,\n return;\n }\n if (progress > pack_to_stdout)\n- fprintf(stderr, \"Delta compression using %d threads.\\n\",\n+ fprintf(stderr, \"Delta compression using up to %d threads.\\n\",\n delta_search_threads);\n\n /* Partition the work amongst work threads. */\n"},{"id":"110505","messageId":"alpine.LFD.2.00.0904052311020.6741@xanadu.home","threadId":"18715","inReplyTo":"449c10960904051934l54d2d504w1af867dc53ef7dd7@mail.gmail.com","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-04-06T03:14:16Z","receivedAt":"2009-04-06T03:14:16Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 5 Apr 2009, Dan McGee wrote:\n\n> On Sun, Apr 5, 2009 at 9:31 PM, Nicolas Pitre wrote:\n> > The number currently displayed has real meaning: this is the number of\n> > threads git is allowed to use.  The number of threads it will actually\n> > use is variable and it changes with time.\n> \n> Would something like this be more ideal then? I wouldn't be so\n> persistent here if the current text wasn't misleading in a case like\n> the following:\n> \n> dmcgee@galway ~/projects/devtools (master)\n> $ git push origin\n> Counting objects: 13, done.\n> Delta compression using 4 threads.\n> Compressing objects: 100% (8/8), done.\n> Writing objects: 100% (8/8), 1.28 KiB, done.\n> Total 8 (delta 6), reused 0 (delta 0)\n> To archlinux.org:/srv/projects/git/devtools.git\n> bcb0e39..ea73c2b master -> master\n> \n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 9fc3b35..99181fd 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -1612,7 +1612,7 @@ static void ll_find_deltas(struct object_entry\n> **list, unsigned list_size,\n> return;\n> }\n> if (progress > pack_to_stdout)\n> - fprintf(stderr, \"Delta compression using %d threads.\\n\",\n> + fprintf(stderr, \"Delta compression using up to %d threads.\\n\",\n> delta_search_threads);\n> \n> /* Partition the work amongst work threads. */\n\nThis I have absolutely no issue with.\n\nAcked-by: Nicolas Pitre \n\n\nNicolas\n"},{"id":"110795","messageId":"7v4owzr72g.fsf@gitster.siamese.dyndns.org","threadId":"18715","inReplyTo":"alpine.LFD.2.00.0904052311020.6741@xanadu.home","subject":"Re: [PATCH 2/2] pack-objects: report actual number of threads to be used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T06:30:14Z","receivedAt":"2009-04-08T06:30:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n>> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n>> index 9fc3b35..99181fd 100644\n>> --- a/builtin-pack-objects.c\n>> +++ b/builtin-pack-objects.c\n>> @@ -1612,7 +1612,7 @@ static void ll_find_deltas(struct object_entry\n>> **list, unsigned list_size,\n>> return;\n>> }\n>> if (progress > pack_to_stdout)\n>> - fprintf(stderr, \"Delta compression using %d threads.\\n\",\n>> + fprintf(stderr, \"Delta compression using up to %d threads.\\n\",\n>> delta_search_threads);\n>> \n>> /* Partition the work amongst work threads. */\n>\n> This I have absolutely no issue with.\n>\n> Acked-by: Nicolas Pitre \n\nI do not have problem with the wording either, but then the commit log\nmessage needs to change, I think. Care to re-submit?\n"},{"id":"110926","messageId":"1239291939-613-1-git-send-email-dpmcgee@gmail.com","threadId":"18715","inReplyTo":"7v4owzr72g.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Update delta compression message to be less misleading","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2009-04-09T15:45:39Z","receivedAt":"2009-04-09T15:45:39Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"Signed-off-by: Dan McGee \n---\n builtin-pack-objects.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 9fc3b35..99181fd 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1612,7 +1612,7 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\treturn;\n \t}\n \tif (progress > pack_to_stdout)\n-\t\tfprintf(stderr, \"Delta compression using %d threads.\\n\",\n+\t\tfprintf(stderr, \"Delta compression using up to %d threads.\\n\",\n \t\t\t\tdelta_search_threads);\n \n \t/* Partition the work amongst work threads. */\n-- \n1.6.2.2\n"},{"id":"111080","messageId":"7veivzdmed.fsf@gitster.siamese.dyndns.org","threadId":"18715","inReplyTo":"1239291939-613-1-git-send-email-dpmcgee@gmail.com","subject":"Re: [PATCH] Update delta compression message to be less misleading","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-11T19:24:10Z","receivedAt":"2009-04-11T19:24:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dan McGee writes:\n\n> Signed-off-by: Dan McGee \n\nEmpty log message?\n\nI am scratching my head because this came as a reply to:\n\n ...\n > This I have absolutely no issue with.\n >\n > Acked-by: Nicolas Pitre \n\n I do not have problem with the wording either, but then the commit log\n message needs to change, I think. Care to re-submit?\n\nand your original read quite nicely:\n\n Subject: [PATCH 2/2] pack-objects: report actual number of threads to be used\n Date: Sat, 4 Apr 2009 11:59:56 -0500\n Message-ID: <1238864396-8964-2-git-send-email-dpmcgee@gmail.com>\n\n In the case of a small repository, pack-objects is smart enough to not start\n more threads than necessary. However, the output to the user always reports\n the value of the pack.threads configuration and not the real number of\n threads to be used. This is easily fixed by moving the printing of the\n message after we have partitioned our work.\n\n (pack.threads is on autodetect and would be set to 4)\n $ git-repack -a -d -f\n Counting objects: 55, done.\n Delta compression using 2 threads.\n Compressing objects: 100% (48/48), done.\n Writing objects: 100% (55/55), done.\n Total 55 (delta 10), reused 0 (delta 0)\n\n Signed-off-by: Dan McGee \n\nI was expecting to see something with a similar structure. An updated\ntitle, an introductory text and the problem description, and the\ndescription of the solution. Then \"Acked-by:\" you already received.\n\nThe new title looks correct, the problem description from the original\nshould still apply, but the solution is different, and that needs to\nchange.\n\nI've committed it like this:\n\n Author: Dan McGee \n Date: Thu Apr 9 10:45:39 2009 -0500\n\n Update delta compression message to be less misleading\n \n In the case of a small repository, pack-objects is smart enough to not\n start more threads than necessary. However, the output to the user always\n reports the value of the pack.threads configuration and not the real\n number of threads to be used.\n \n Signed-off-by: Dan McGee \n Acked-by: Nicolas Pitre \n Signed-off-by: Junio C Hamano \n\nThanks.\n"},{"id":"111088","messageId":"449c10960904111242y38279c9cxc0a464255c2aa4db@mail.gmail.com","threadId":"18715","inReplyTo":"7veivzdmed.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Update delta compression message to be less misleading","fromName":"Dan McGee","fromEmail":"dpmcgee@gmail.com","sentAt":"2009-04-11T19:42:08Z","receivedAt":"2009-04-11T19:42:08Z","isPatch":true,"sender":{"key":"dpmcgee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/265817?v=4"},"body":"On Sat, Apr 11, 2009 at 2:24 PM, Junio C Hamano wrote:\n> Dan McGee writes:\n>\n>> Signed-off-by: Dan McGee \n>\n> Empty log message?\n>\n> I am scratching my head because this came as a reply to:\n>\n>    ...\n>    > This I have absolutely no issue with.\n>    >\n>    > Acked-by: Nicolas Pitre \n>\n>    I do not have problem with the wording either, but then the commit log\n>    message needs to change, I think.  Care to re-submit?\n>\n> and your original read quite nicely:\n>\n>    Subject: [PATCH 2/2] pack-objects: report actual number of threads to be used\n>    Date: Sat,  4 Apr 2009 11:59:56 -0500\n>    Message-ID: <1238864396-8964-2-git-send-email-dpmcgee@gmail.com>\n>\n>    In the case of a small repository, pack-objects is smart enough to not start\n>    more threads than necessary. However, the output to the user always reports\n>    the value of the pack.threads configuration and not the real number of\n>    threads to be used. This is easily fixed by moving the printing of the\n>    message after we have partitioned our work.\n>\n>    (pack.threads is on autodetect and would be set to 4)\n>    $ git-repack -a -d -f\n>    Counting objects: 55, done.\n>    Delta compression using 2 threads.\n>    Compressing objects: 100% (48/48), done.\n>    Writing objects: 100% (55/55), done.\n>    Total 55 (delta 10), reused 0 (delta 0)\n>\n>    Signed-off-by: Dan McGee \n>\n> I was expecting to see something with a similar structure.  An updated\n> title, an introductory text and the problem description, and the\n> description of the solution.  Then \"Acked-by:\" you already received.\n>\n> The new title looks correct, the problem description from the original\n> should still apply, but the solution is different, and that needs to\n> change.\n>\n> I've committed it like this:\n>\n>    Author: Dan McGee \n>    Date:   Thu Apr 9 10:45:39 2009 -0500\n>\n>    Update delta compression message to be less misleading\n>\n>    In the case of a small repository, pack-objects is smart enough to not\n>    start more threads than necessary. However, the output to the user always\n>    reports the value of the pack.threads configuration and not the real\n>    number of threads to be used.\n>\n>    Signed-off-by: Dan McGee \n>    Acked-by: Nicolas Pitre \n>    Signed-off-by: Junio C Hamano \n>\n> Thanks.\n\nSorry about that. I felt like the patch was a lot more\nself-explanatory now, since it was a text change and not a behavior\nchange. It was my fault for forgetting the Ack.\n\n-Dan\n"}]}