{"thread":{"id":"33621","subject":"\"git grep\" parallelism question","startedAt":"2013-04-26T17:31:40Z","lastAt":"2013-05-05T15:40:01Z","messageCount":16,"participants":["Linus Torvalds","Junio C Hamano","Thomas Rast","Ramkumar Ramachandra","John Keeping","Jeff King","Pete Wyckoff"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"215603","messageId":"CA+55aFxY2PJ+L=vCfvQ39UGBr7E6m5q76hO=z3Mqm6vTQmmMbw@mail.gmail.com","threadId":"33621","inReplyTo":null,"subject":"\"git grep\" parallelism question","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2013-04-26T17:31:40Z","receivedAt":"2013-04-26T17:31:40Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"Since I reboot fairly regularly to test new kernels, I don't *always*\nhave the kernel source tree in my caches, so I still care about some\ncold-cache performance. And \"git grep\" tends to be the most noticeable\none.\n\nNow, I have a SSD, and even the cold-cache case takes just five\nseconds or so, but that's still somethng I react to, since the normal\n\"kernel tree in cache\" case ends up bring close enough to\ninstantaneous (half a second) that then when it takes longer I react\nto it.\n\nAnd I started thinking about it, and our \"git grep\" parallelism seems\nto be limited to 8.\n\nWhich is fine most of the time for CPU parallelism (although maybe\nsome people with big machines would prefer higher numbers), but for IO\nparallelism I thought that maybe we'd like a higher number...\n\nSo I tried it out, and with THREADS set to 32, I get a roughly 15%\nperformance boost for the cold-cache case (the error bar is high\nenough to not give a very precise number, but I see it going from\n~4.8-4.9s on my machine down to 4.2..4.6s).\n\nThat's on an SSD, though - the performance implications might be very\ndifferent for other use cases (NFS would likely prefer higher IO\nparallelism and might show bigger improvement, while a rotational disk\nmight end up just thrashing more)\n\nIs this a big deal? Probably not. But I did react to how annoying it\nwas to set the parallelism factor (recompile git with a new number).\nWouldn't it be lovely if it was slightly smarter (something more akin\nto the index preloading that takes number of files into account) or at\nleast allowed people to set the parallelism explicitly with a command\nline switch?\n\nRight now it disables the parallel grep entirely for UP, for example.\nWhich makes perfect sense if grep is all about CPU use. But even UP\nmight improve from parallel IO..\n\n Linus\n"},{"id":"215610","messageId":"7vr4hxw2mp.fsf@alter.siamese.dyndns.org","threadId":"33621","inReplyTo":"CA+55aFxY2PJ+L=vCfvQ39UGBr7E6m5q76hO=z3Mqm6vTQmmMbw@mail.gmail.com","subject":"Re: \"git grep\" parallelism question","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-26T18:47:10Z","receivedAt":"2013-04-26T18:47:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Wouldn't it be lovely if it was slightly smarter (something more akin\n> to the index preloading that takes number of files into account) or at\n> least allowed people to set the parallelism explicitly with a command\n> line switch?\n\nYeah, a reasonable starting point for auto-tuning may be to use the\nsame kind of parameters and heuristics (i.e. max parallel of 20\nthreads, assume a cost to use an extra thread is the same as running\n500 greps), and then tweak them (for example, thread cost of 500 may\nbe reasonable for lstat() but it would be way too big for grep()).\n\nThe real issue may be that we do not have a good estimate of how\nmany paths are involved in the request before starting these\nthreads, though.\n"},{"id":"215613","messageId":"CA+55aFw+6pL5DoEPsPZpJCAbqEGaWYYKcdjZzbsHVzSSMrQmww@mail.gmail.com","threadId":"33621","inReplyTo":"7vr4hxw2mp.fsf@alter.siamese.dyndns.org","subject":"Re: \"git grep\" parallelism question","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2013-04-26T18:54:09Z","receivedAt":"2013-04-26T18:54:09Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Fri, Apr 26, 2013 at 11:47 AM, Junio C Hamano wrote:\n>\n> The real issue may be that we do not have a good estimate of how\n> many paths are involved in the request before starting these\n> threads, though.\n\nYes. Also, I'm not sure if the 15% possible improvement on my SSD case\nis even worth it for something that in the end isn't necessarily the\ncommon case. I *suspect* that it might be a much bigger deal on NFS\n(IO parallelism really does end up being a big deal sometimes, and\ncaching tends to be less aggressive too), but on rotational media it\nmight be much less clear, or even a loss..\n\nAre there people out there who use \"git grep\" over NFS and have been\nunhappy with performance? If are willing to recompile git with a\ndifferent THREAD value in builtin/grep.c, then on a Linux client you\ncan try\n\n echo 3 > /proc/sys/vm/drop_caches\n\nto largely force cold-cache behavior for testing (I say \"largely\",\nbecause it won't drop busy/dirty pages, but for \"git grep\" kind of\nloads it should be good).\n\nOf course, you need root for it, so..\n\n Linus\n"},{"id":"215618","messageId":"7vip39w14d.fsf@alter.siamese.dyndns.org","threadId":"33621","inReplyTo":"CA+55aFw+6pL5DoEPsPZpJCAbqEGaWYYKcdjZzbsHVzSSMrQmww@mail.gmail.com","subject":"Re: \"git grep\" parallelism question","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-26T19:19:46Z","receivedAt":"2013-04-26T19:19:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Yes. Also, I'm not sure if the 15% possible improvement on my SSD case\n> is even worth it for something that in the end isn't necessarily the\n> common case.\n\nCold cache being uncommon case would be forever true but more and\nmore people are on SSD, and 15% is not a trivial improvement.\n\n> Are there people out there who use \"git grep\" over NFS and have been\n> unhappy with performance? If are willing to recompile git with a\n> different THREAD value in builtin/grep.c,...\n\nOK, you have to recompile at least once for experiment, so perhaps\nbuilding the test binary with this patch may help.\n\n builtin/grep.c | 30 ++++++++++++++++++++++--------\n 1 file changed, 22 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 159e65d..f635cd5 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -26,12 +26,14 @@ static char const * const grep_usage[] = {\n static int use_threads = 1;\n \n #ifndef NO_PTHREADS\n-#define THREADS 8\n-static pthread_t threads[THREADS];\n+#define MAX_THREADS 100\n+static int num_threads = 8;\n+static pthread_t threads[MAX_THREADS];\n \n-/* We use one producer thread and THREADS consumer\n- * threads. The producer adds struct work_items to 'todo' and the\n- * consumers pick work items from the same array.\n+/*\n+ * We use one producer thread and THREADS consumer threads. The\n+ * producer adds struct work_items to 'todo' and the consumers pick\n+ * work items from the same array.\n */\n struct work_item {\n \tstruct grep_source source;\n@@ -205,7 +207,7 @@ static void start_threads(struct grep_opt *opt)\n \t\tstrbuf_init(&todo[i].out, 0);\n \t}\n \n-\tfor (i = 0; i < ARRAY_SIZE(threads); i++) {\n+\tfor (i = 0; i < num_threads; i++) {\n \t\tint err;\n \t\tstruct grep_opt *o = grep_opt_dup(opt);\n \t\to->output = strbuf_out;\n@@ -237,7 +239,7 @@ static int wait_all(void)\n \tpthread_cond_broadcast(&cond_add);\n \tgrep_unlock();\n \n-\tfor (i = 0; i < ARRAY_SIZE(threads); i++) {\n+\tfor (i = 0; i < num_threads; i++) {\n \t\tvoid *h;\n \t\tpthread_join(threads[i], &h);\n \t\thit |= (int) (intptr_t) h;\n@@ -636,6 +638,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint i;\n \tint dummy;\n \tint use_index = 1;\n+\tint num_threads_explicit = -1;\n \tint pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;\n \n \tstruct option options[] = {\n@@ -743,6 +746,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"allow calling of grep(1) (ignored by this build)\")),\n \t\t{ OPTION_CALLBACK, 0, \"help-all\", &options, NULL, N_(\"show usage\"),\n \t\t PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },\n+\t\tOPT_INTEGER(0, \"threads\", &num_threads_explicit,\n+\t\t\t N_(\"use threads when searching\")),\n \t\tOPT_END()\n \t};\n \n@@ -773,6 +778,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_NO_INTERNAL_HELP);\n \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n \n+\tif (MAX_THREADS <= num_threads_explicit) {\n+\t\twarning(\"limiting --threads to %d\", MAX_THREADS);\n+\t\tnum_threads = MAX_THREADS;\n+\t} else if (num_threads_explicit < 0) {\n+\t\t; /* keep num_threads to compiled-in default */\n+\t} else {\n+\t\tnum_threads = num_threads_explicit;\n+\t}\n+\n \tif (use_index && !startup_info->have_repository)\n \t\t/* die the same way as if we did it at the beginning */\n \t\tsetup_git_directory();\n@@ -834,7 +848,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t}\n \n #ifndef NO_PTHREADS\n-\tif (list.nr || cached || online_cpus() == 1)\n+\tif ((list.nr || cached || online_cpus() == 1) && num_threads_explicit < 0)\n \t\tuse_threads = 0;\n #else\n \tuse_threads = 0;\n"},{"id":"215635","messageId":"CA+55aFx1t_MT+20Bbkse-wHeLz8E06yqaOhbb12GzHNDrE2tWA@mail.gmail.com","threadId":"33621","inReplyTo":"7vip39w14d.fsf@alter.siamese.dyndns.org","subject":"Re: \"git grep\" parallelism question","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2013-04-26T20:31:41Z","receivedAt":"2013-04-26T20:31:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Fri, Apr 26, 2013 at 12:19 PM, Junio C Hamano wrote:\n>\n> OK, you have to recompile at least once for experiment, so perhaps\n> building the test binary with this patch may help.\n\nSo here's my experiment on my machine with this patch and the kernel\ntree. First I did the warm-cache case:\n\n for i in 1 4 8 16 32 64\n do\n echo $i:\n for j in 1 2 3 4\n do\n t=$(sh -c \"time git grep --threads=$i hjahsja\" 2>&1 | grep real)\n echo $i $t\n done\n done\n\nand the numbers are pretty stable, here's just he summary (best of\nfour tries for each case):\n\n 1 real 0m0.598s\n 4 real 0m0.253s\n 8 real 0m0.235s\n 16 real 0m0.269s\n 32 real 0m0.412s\n 64 real 0m0.420s\n\nso for this machine, 8 threads (our old number) is actually optimal\neven if it has just four cores (actually, two cores with HT). I\nsuspect it's just because the load is slightly unbalanced, so a few\nextra threads helps. Looks like anything in the 4-16 thread range is\nok, though. But 32 threads is clearly too much.\n\nThen I did the exact same thing, but with the \"echo 3 >\n/proc/sys/vm/drop_caches\" just before the timing of the git grep.\nAgain, summarizing (best-of-four number, the variation wasn't that\nbig):\n\n 1 real 0m17.866s\n 4 real 0m6.367s\n 8 real 0m4.855s\n 16 real 0m4.307s\n 32 real 0m4.153s\n 64 real 0m4.128s\n\nhere, the numbers continue to improve up to 64 threads, although the\ndifference between 32 and 64 is starting to be in the noise. I suspect\nit's a combination of better IO overlap (although not all SSD's will\neven improve all that much from overlapping IO past a certain point)\nand probably a more noticeable imbalance between threads.\n\nAnyway, for *my* machine and for *this* particular load, I'd say that\nwe're already pretty close to optimal. I did some trials just to see,\nbut the best hot-cache numbers were fairly reliably for 7 or 8\nthreads.\n\nLooking at the numbers, I can't really convince myself that it would\nbe worth it to do (say) 12 threads on this machine. Yes, the\ncold-cache case improves from the 8-thread case (best-of-four for 12\nthreads: 0m4.467s), but the hot-cache case has gotten sufficiently\nworse (0m0.251s) that I'm not sure..\n\nOf course, in *absolute* numbers the cold-cache case is so much slower\nthat a half-second improvement from going to 16 threads might be\nconsidered worth it, because while the the hot-cache case gets worse,\nwe may just not care because it's fast enough that it's not\nnoticeable.\n\nAnyway, I think your patch is good if for no other reason that it\nallows this kind of testing, but at least for my machine, clearly the\ncurrent default of eight threads is actually \"good enough\". Maybe\nsomebody with a very different machine might want to run the above\nscript and see if how sensitive other machines are to this parameter..\n\n Linus\n"},{"id":"215706","messageId":"87mwskf5nj.fsf@hexa.v.cablecom.net","threadId":"33621","inReplyTo":"CA+55aFx1t_MT+20Bbkse-wHeLz8E06yqaOhbb12GzHNDrE2tWA@mail.gmail.com","subject":"Re: \"git grep\" parallelism question","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-27T13:46:08Z","receivedAt":"2013-04-27T13:46:08Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Linus Torvalds writes:\n\n> Anyway, I think your patch is good if for no other reason that it\n> allows this kind of testing, but at least for my machine, clearly the\n> current default of eight threads is actually \"good enough\". Maybe\n> somebody with a very different machine might want to run the above\n> script and see if how sensitive other machines are to this parameter..\n\nI think the last time Duy, Peff, me and others looked into grep\nthreading (which was a while ago) we basically reached the conclusion\nthat it's very unstable across machines.\n\nBack then I tested things on a 2x6-core Xeon OS X machine and there the\nperformance (for the hot-cache, worktree case, which should parallelize\nnicely) flatlines at 5 threads for no apparent reason.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215851","messageId":"CALkWK0k6Gi_J6nDbrGPxDMmWC73CHXdj7a5ugC15YVrrycP=hA@mail.gmail.com","threadId":"33621","inReplyTo":"CA+55aFx1t_MT+20Bbkse-wHeLz8E06yqaOhbb12GzHNDrE2tWA@mail.gmail.com","subject":"Re: \"git grep\" parallelism question","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-29T14:05:01Z","receivedAt":"2013-04-29T14:05:01Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Linus Torvalds wrote:\n> Anyway, I think your patch is good if for no other reason that it\n> allows this kind of testing, but at least for my machine, clearly the\n> current default of eight threads is actually \"good enough\". Maybe\n> somebody with a very different machine might want to run the above\n> script and see if how sensitive other machines are to this parameter..\n\nI have four cores with HT, so I expect similar gains from CPU\nparallelism (in the hot-cache case). And the test results meet this\nexpectation:\n\n1 real 0m1.287s\n2 real 0m0.635s\n4 real 0m0.460s\n8 real 0m0.413s\n16 real 0m0.443s\n32 real 0m0.715s\n\nI have a rotating hard disk, and expect the IO parallelism (cold-cache\ncase) benefits to peak at a larger number of threads (than in your SSD\ncase). These are the test results:\n\n1 real 0m28.566s\n4 real 0m20.361s\n8 real 0m16.990s\n16 real 0m15.278s\n32 real 0m13.710s\n64 real 0m12.405s\n128 real 0m11.913s\n256 real 0m11.759s\n512 real 0m12.022s\n\nIt looks like 128~256 threads is the sweet spot.\n\nIt's impossible for git to determine if the cache is hot or cold,\ncorrect? Cold cache is a very rare case, and I'm not sure how we can\noptimize specifically for that case. So, I'm not sure how we can\nimprove grep.\n\nOn a related note, one place that IO parallelism can provide massive\nbenefits is in executing shell scripts. Accordingly, I always use the\nfollowing commands to compile and test git respectively:\n\n make -j 8 CFLAGS=\"-g -O0 -Wall\"\n make -j 8 DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS=\"-j 16\" test\n\ni.e. always use 8 threads when the task is known to be CPU intensive,\nand always use 16 threads when the task is known to be IO intensive.\n"},{"id":"215862","messageId":"20130429161814.GJ472@serenity.lan","threadId":"33621","inReplyTo":"CALkWK0k6Gi_J6nDbrGPxDMmWC73CHXdj7a5ugC15YVrrycP=hA@mail.gmail.com","subject":"Re: \"git grep\" parallelism question","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-29T16:18:14Z","receivedAt":"2013-04-29T16:18:14Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Apr 29, 2013 at 07:35:01PM +0530, Ramkumar Ramachandra wrote:\n> On a related note, one place that IO parallelism can provide massive\n> benefits is in executing shell scripts. Accordingly, I always use the\n> following commands to compile and test git respectively:\n> \n> make -j 8 CFLAGS=\"-g -O0 -Wall\"\n> make -j 8 DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS=\"-j 16\" test\n> \n> i.e. always use 8 threads when the task is known to be CPU intensive,\n> and always use 16 threads when the task is known to be IO intensive.\n\nOn this tangent, I recently added a TEST_OUTPUT_DIRECTORY line to my\nconfig.mak which points into a tmpfs mount. Keeping all of the test\nrepositories in RAM makes the tests significantly faster for me and\nworks nicely when you have the patches in jk/test-output (without those\npatches the individual tests work but the reporting of aggregate results\ndoesn't).\n"},{"id":"215887","messageId":"877gjldxid.fsf@hexa.v.cablecom.net","threadId":"33621","inReplyTo":"20130429161814.GJ472@serenity.lan","subject":"Re: \"git grep\" parallelism question","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-29T18:04:10Z","receivedAt":"2013-04-29T18:04:10Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"John Keeping writes:\n\n> On Mon, Apr 29, 2013 at 07:35:01PM +0530, Ramkumar Ramachandra wrote:\n>> On a related note, one place that IO parallelism can provide massive\n>> benefits is in executing shell scripts. Accordingly, I always use the\n>> following commands to compile and test git respectively:\n>> \n>> make -j 8 CFLAGS=\"-g -O0 -Wall\"\n>> make -j 8 DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS=\"-j 16\" test\n>> \n>> i.e. always use 8 threads when the task is known to be CPU intensive,\n>> and always use 16 threads when the task is known to be IO intensive.\n>\n> On this tangent, I recently added a TEST_OUTPUT_DIRECTORY line to my\n> config.mak which points into a tmpfs mount. Keeping all of the test\n> repositories in RAM makes the tests significantly faster for me and\n> works nicely when you have the patches in jk/test-output (without those\n> patches the individual tests work but the reporting of aggregate results\n> doesn't).\n\nBut that's been possible for quite some time now, using --root, or am I\nmissing something?\n\n(Not that the fix as such is a bad idea, but other readers might not\nwant to wait for it to hit master.)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215888","messageId":"20130429180857.GK472@serenity.lan","threadId":"33621","inReplyTo":"877gjldxid.fsf@hexa.v.cablecom.net","subject":"Re: \"git grep\" parallelism question","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-29T18:08:57Z","receivedAt":"2013-04-29T18:08:57Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Apr 29, 2013 at 08:04:10PM +0200, Thomas Rast wrote:\n> John Keeping writes:\n> \n> > On Mon, Apr 29, 2013 at 07:35:01PM +0530, Ramkumar Ramachandra wrote:\n> >> On a related note, one place that IO parallelism can provide massive\n> >> benefits is in executing shell scripts. Accordingly, I always use the\n> >> following commands to compile and test git respectively:\n> >> \n> >> make -j 8 CFLAGS=\"-g -O0 -Wall\"\n> >> make -j 8 DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS=\"-j 16\" test\n> >> \n> >> i.e. always use 8 threads when the task is known to be CPU intensive,\n> >> and always use 16 threads when the task is known to be IO intensive.\n> >\n> > On this tangent, I recently added a TEST_OUTPUT_DIRECTORY line to my\n> > config.mak which points into a tmpfs mount. Keeping all of the test\n> > repositories in RAM makes the tests significantly faster for me and\n> > works nicely when you have the patches in jk/test-output (without those\n> > patches the individual tests work but the reporting of aggregate results\n> > doesn't).\n> \n> But that's been possible for quite some time now, using --root, or am I\n> missing something?\n\nNo, I was the one missing something (--root to be precise). But with\nTEST_OUTPUT_DIRECTORY you also get the result files in your temporary\nlocation, not just the trash directory.\n"},{"id":"215926","messageId":"7v1u9tgeov.fsf@alter.siamese.dyndns.org","threadId":"33621","inReplyTo":"20130429180857.GK472@serenity.lan","subject":"Re: \"git grep\" parallelism question","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-29T22:22:24Z","receivedAt":"2013-04-29T22:22:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> On Mon, Apr 29, 2013 at 08:04:10PM +0200, Thomas Rast wrote:\n>> John Keeping writes:\n>> \n>> > On Mon, Apr 29, 2013 at 07:35:01PM +0530, Ramkumar Ramachandra wrote:\n>> >> On a related note, one place that IO parallelism can provide massive\n>> >> benefits is in executing shell scripts. Accordingly, I always use the\n>> >> following commands to compile and test git respectively:\n>> >> \n>> >> make -j 8 CFLAGS=\"-g -O0 -Wall\"\n>> >> make -j 8 DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS=\"-j 16\" test\n>> >> \n>> >> i.e. always use 8 threads when the task is known to be CPU intensive,\n>> >> and always use 16 threads when the task is known to be IO intensive.\n>> >\n>> > On this tangent, I recently added a TEST_OUTPUT_DIRECTORY line to my\n>> > config.mak which points into a tmpfs mount. Keeping all of the test\n>> > repositories in RAM makes the tests significantly faster for me and\n>> > works nicely when you have the patches in jk/test-output (without those\n>> > patches the individual tests work but the reporting of aggregate results\n>> > doesn't).\n>> \n>> But that's been possible for quite some time now, using --root, or am I\n>> missing something?\n>\n> No, I was the one missing something (--root to be precise). But with\n> TEST_OUTPUT_DIRECTORY you also get the result files in your temporary\n> location, not just the trash directory.\n\nWith your patch, doesn't \"tXXXX-*.sh --root $there\" automatically\nuse the fast $there temporary location as the result depot, too?\nIf it doesn't with the current code, shouldn't it?\n \n"},{"id":"215973","messageId":"20130430080848.GP472@serenity.lan","threadId":"33621","inReplyTo":"7v1u9tgeov.fsf@alter.siamese.dyndns.org","subject":"Re: \"git grep\" parallelism question","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-30T08:08:49Z","receivedAt":"2013-04-30T08:08:49Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Apr 29, 2013 at 03:22:24PM -0700, Junio C Hamano wrote:\n> John Keeping writes:\n> > No, I was the one missing something (--root to be precise). But with\n> > TEST_OUTPUT_DIRECTORY you also get the result files in your temporary\n> > location, not just the trash directory.\n> \n> With your patch, doesn't \"tXXXX-*.sh --root $there\" automatically\n> use the fast $there temporary location as the result depot, too?\n\nNo, the current code uses:\n\n $TEST_OUTPUT_DIRECTORY/$root/trash\\ directory.tXXXX\n\nwhere we don't prepend $TEST_OUTPUT_DIRECTORY/ if $root is absolute.\n\n> If it doesn't with the current code, shouldn't it?\n\nI think the current behaviour is fine and the two options complement\neach other.\n\nTEST_OUTPUT_DIRECTORY is something you set once and forget about which\nsays \"all of the test output should go over here\", whereas --root is\npassed to a specific test and says \"put your output here\" but does not\naffect the result aggregation which is not specific to that test.\n\nNote that setting TEST_OUTPUT_DIRECTORY in config.mak affects all tests\nno matter how you run them (via make or as ./tXXXX-yyyy.sh) whereas\nsetting --root=... in GIT_TEST_OPTS only affect tests run via make.\n"},{"id":"215994","messageId":"20130430155939.GA31881@sigill.intra.peff.net","threadId":"33621","inReplyTo":"20130430080848.GP472@serenity.lan","subject":"Re: \"git grep\" parallelism question","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-30T15:59:39Z","receivedAt":"2013-04-30T15:59:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 30, 2013 at 09:08:49AM +0100, John Keeping wrote:\n\n> > With your patch, doesn't \"tXXXX-*.sh --root $there\" automatically\n> > use the fast $there temporary location as the result depot, too?\n> \n> No, the current code uses:\n> \n> $TEST_OUTPUT_DIRECTORY/$root/trash\\ directory.tXXXX\n> \n> where we don't prepend $TEST_OUTPUT_DIRECTORY/ if $root is absolute.\n> \n> > If it doesn't with the current code, shouldn't it?\n> \n> I think the current behaviour is fine and the two options complement\n> each other.\n> \n> TEST_OUTPUT_DIRECTORY is something you set once and forget about which\n> says \"all of the test output should go over here\", whereas --root is\n> passed to a specific test and says \"put your output here\" but does not\n> affect the result aggregation which is not specific to that test.\n\nThe original intent of \"--root\" (and how I use it) is to set and forget\nit, too, via GIT_TEST_OPTS. I intentionally didn't move test results\nwith it, because to me the point was a pure optimization: put the trash\ndirectories on a faster disk, and leave everything else identical. With\n\"--root\", any scripts which later want to look at test-results will find\nthem in the usual place.\n\nYour patch updates all of the in-tree spots which look at the results,\nbut any third-party scripts would need to take it into account, too\n(though I have no idea if any such scripts even exist).\n\nI'm curious if there is a good reason to want to move the results. Some\npossibilities I can think of are:\n\n 1. More optimization, as results are written to the faster filesystem.\n I doubt this is noticeable, though, as the amount of data written\n is relatively small compared to the tests themselves (which are\n constantly creating and deleting repos).\n\n 2. You can run tests in a read-only git checkout. I'm not sure how\n useful that is, though, since you would already need to compile\n git.\n\n 3. You could have multiple sets of test results to keep or compare.\n I'd think you'd want to keep the built versions of git around, too,\n though. Which would mean that a full checkout like git-new-workdir\n would be a much simpler way to accomplish the same thing.\n\nSo I'm not against TEST_OUTPUT_DIRECTORY as a concept, but I'm having\ntrouble seeing how it is more useful than \"--root\".\n\n> Note that setting TEST_OUTPUT_DIRECTORY in config.mak affects all tests\n> no matter how you run them (via make or as ./tXXXX-yyyy.sh) whereas\n> setting --root=... in GIT_TEST_OPTS only affect tests run via make.\n\nI actually consider that a feature of \"--root\". When I run \"make test\"\neverything happens fast. When I run the script manually (which is\nusually because I'm debugging), the trash directory appears in the\ncurrent directory, so I can easily investigate it. And if you are\nrunning a single test, the performance impact is usually negligible\n(where you really notice it is when running \"make -j32 test\").\n\n-Peff\n"},{"id":"215997","messageId":"20130430161207.GQ472@serenity.lan","threadId":"33621","inReplyTo":"20130430155939.GA31881@sigill.intra.peff.net","subject":"Re: \"git grep\" parallelism question","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-30T16:12:08Z","receivedAt":"2013-04-30T16:12:08Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Apr 30, 2013 at 11:59:39AM -0400, Jeff King wrote:\n> On Tue, Apr 30, 2013 at 09:08:49AM +0100, John Keeping wrote:\n> \n> > > With your patch, doesn't \"tXXXX-*.sh --root $there\" automatically\n> > > use the fast $there temporary location as the result depot, too?\n> > \n> > No, the current code uses:\n> > \n> > $TEST_OUTPUT_DIRECTORY/$root/trash\\ directory.tXXXX\n> > \n> > where we don't prepend $TEST_OUTPUT_DIRECTORY/ if $root is absolute.\n> > \n> > > If it doesn't with the current code, shouldn't it?\n> > \n> > I think the current behaviour is fine and the two options complement\n> > each other.\n> > \n> > TEST_OUTPUT_DIRECTORY is something you set once and forget about which\n> > says \"all of the test output should go over here\", whereas --root is\n> > passed to a specific test and says \"put your output here\" but does not\n> > affect the result aggregation which is not specific to that test.\n> \n> The original intent of \"--root\" (and how I use it) is to set and forget\n> it, too, via GIT_TEST_OPTS. I intentionally didn't move test results\n> with it, because to me the point was a pure optimization: put the trash\n> directories on a faster disk, and leave everything else identical. With\n> \"--root\", any scripts which later want to look at test-results will find\n> them in the usual place.\n> \n> Your patch updates all of the in-tree spots which look at the results,\n> but any third-party scripts would need to take it into account, too\n> (though I have no idea if any such scripts even exist).\n> \n> I'm curious if there is a good reason to want to move the results. Some\n> possibilities I can think of are:\n> \n> 1. More optimization, as results are written to the faster filesystem.\n> I doubt this is noticeable, though, as the amount of data written\n> is relatively small compared to the tests themselves (which are\n> constantly creating and deleting repos).\n> \n> 2. You can run tests in a read-only git checkout. I'm not sure how\n> useful that is, though, since you would already need to compile\n> git.\n> \n> 3. You could have multiple sets of test results to keep or compare.\n> I'd think you'd want to keep the built versions of git around, too,\n> though. Which would mean that a full checkout like git-new-workdir\n> would be a much simpler way to accomplish the same thing.\n> \n> So I'm not against TEST_OUTPUT_DIRECTORY as a concept, but I'm having\n> trouble seeing how it is more useful than \"--root\".\n\nI think the original intent of TEST_OUTPUT_DIRECTORY was to allow other\nusers of the test framework (in contrib/ or the performance tests) to\nput their output in a sensible place for those tests, like you describe\nbelow.\n\nThe patch being discussed here [1] just makes sure that it applies\nto everything - previously it was applied to test-results/\ninconsistently; test-lib.sh used TEST_OUTPUT_DIRECTORY but the makefile\ndidn't. So we haven't actually changed where test-results/ live as a\nresult of this change, just where the makefile looks in order to display\nthe aggregate results and clean them up.\n\n> > Note that setting TEST_OUTPUT_DIRECTORY in config.mak affects all tests\n> > no matter how you run them (via make or as ./tXXXX-yyyy.sh) whereas\n> > setting --root=... in GIT_TEST_OPTS only affect tests run via make.\n> \n> I actually consider that a feature of \"--root\". When I run \"make test\"\n> everything happens fast. When I run the script manually (which is\n> usually because I'm debugging), the trash directory appears in the\n> current directory, so I can easily investigate it. And if you are\n> running a single test, the performance impact is usually negligible\n> (where you really notice it is when running \"make -j32 test\").\n\nThis confirms to me that the patch as it currently stands is correct: we\nhave made TEST_OUTPUT_DIRECTORY consistent and --root still works as\nbefore.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/222555\n"},{"id":"215998","messageId":"20130430161444.GA32748@sigill.intra.peff.net","threadId":"33621","inReplyTo":"20130430161207.GQ472@serenity.lan","subject":"Re: \"git grep\" parallelism question","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-30T16:14:45Z","receivedAt":"2013-04-30T16:14:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 30, 2013 at 05:12:08PM +0100, John Keeping wrote:\n\n> > So I'm not against TEST_OUTPUT_DIRECTORY as a concept, but I'm having\n> > trouble seeing how it is more useful than \"--root\".\n> \n> I think the original intent of TEST_OUTPUT_DIRECTORY was to allow other\n> users of the test framework (in contrib/ or the performance tests) to\n> put their output in a sensible place for those tests, like you describe\n> below.\n> \n> The patch being discussed here [1] just makes sure that it applies\n> to everything - previously it was applied to test-results/\n> inconsistently; test-lib.sh used TEST_OUTPUT_DIRECTORY but the makefile\n> didn't. So we haven't actually changed where test-results/ live as a\n> result of this change, just where the makefile looks in order to display\n> the aggregate results and clean them up.\n\nAh, I see. Thanks, that was the piece I was missing.\n\n> This confirms to me that the patch as it currently stands is correct: we\n> have made TEST_OUTPUT_DIRECTORY consistent and --root still works as\n> before.\n> \n> [1] http://article.gmane.org/gmane.comp.version-control.git/222555\n\nYeah, if it is about harmonizing the Makefile and the test scripts, that\nis definitely a bug fix, and the right thing to do.\n\n-Peff\n"},{"id":"216451","messageId":"20130505154001.GA25257@padd.com","threadId":"33621","inReplyTo":"CA+55aFx1t_MT+20Bbkse-wHeLz8E06yqaOhbb12GzHNDrE2tWA@mail.gmail.com","subject":"Re: \"git grep\" parallelism question","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-05-05T15:40:01Z","receivedAt":"2013-05-05T15:40:01Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"torvalds@linux-foundation.org wrote on Fri, 26 Apr 2013 13:31 -0700:\n> Anyway, I think your patch is good if for no other reason that it\n> allows this kind of testing, but at least for my machine, clearly the\n> current default of eight threads is actually \"good enough\". Maybe\n> somebody with a very different machine might want to run the above\n> script and see if how sensitive other machines are to this parameter..\n\nNFS numbers behave as expected: IO concurrency is key.\n\nWARM\n1 real 0m23.147s\n2 real 0m13.913s\n4 real 0m6.958s\n8 real 0m4.104s\n16 real 0m3.588s\n32 real 0m3.212s\n64 real 0m3.173s\n\nCOLD\n1 real 1m36.969s\n2 real 0m51.627s\n4 real 0m32.994s\n8 real 0m25.657s\n16 real 0m21.260s\n32 real 0m18.138s\n64 real 0m17.265s\n\nI am tempted to change the default locally from 8 to 32.\n\n\t\t-- Pete\n"}]}