{"thread":{"id":"36516","subject":"[PATCH] Sleep 1 millisecond in poll() to avoid busy wait","startedAt":"2014-04-28T08:39:31Z","lastAt":"2014-05-08T20:36:57Z","messageCount":18,"participants":["Stepan Kasal","Erik Faye-Lund","Johannes Sixt","Junio C Hamano","Heiko Voigt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"239840","messageId":"20140428083931.GA10257@camelia.ucw.cz","threadId":"36516","inReplyTo":null,"subject":"[PATCH] Sleep 1 millisecond in poll() to avoid busy wait","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-04-28T08:39:31Z","receivedAt":"2014-04-28T08:39:31Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"From: theoleblond <theodore.leblond@gmail.com>\nDate: Wed, 16 May 2012 06:52:49 -0700\n\nI played around with this quite a bit. After trying some more complex\nschemes, I found that what worked best is to just sleep 1 millisecond\nbetween iterations. Though it's a very short time, it still completely\neliminates the busy wait condition, without hurting perf.\n\nThere code uses SleepEx(1, TRUE) to sleep. See this page for a good\ndiscussion of why that is better than calling SwitchToThread, which\nis what was used previously:\nhttp://stackoverflow.com/questions/1383943/switchtothread-vs-sleep1\n\nNote that calling SleepEx(0, TRUE) does *not* solve the busy wait.\n\nThe most striking case was when testing on a UNC share with a large repo,\non a single CPU machine. Without the fix, it took 4 minutes 15 seconds,\nand with the fix it took just 1:08! I think it's because git-upload-pack's\nbusy wait was eating the CPU away from the git process that's doing the\nreal work. With multi-proc, the timing is not much different, but tons of\nCPU time is still wasted, which can be a killer on a server that needs to\ndo bunch of other things.\n\nI also tested the very fast local case, and didn't see any measurable\ndifference. On a big repo with 4500 files, the upload-pack took about 2\nseconds with and without the fix.\n---\n\nThis is one of the patches that lives in msysGit, could it be\naccepted upstream?\nIt modifies the Windows compat function only.\n\nHave a nice day,\n\tStepan\n\n compat/poll/poll.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/poll/poll.c b/compat/poll/poll.c\nindex 31163f2..c5a9a93 100644\n--- a/compat/poll/poll.c\n+++ b/compat/poll/poll.c\n@@ -605,7 +605,9 @@ restart:\n \n   if (!rc && timeout == INFTIM)\n     {\n-      SwitchToThread();\n+      /* Sleep 1 millisecond to avoid busy wait */\n+      SleepEx(1, TRUE);\n+\n       goto restart;\n     }\n \n-- \n1.9.2.msysgit.0.158.g6070cee\n"},{"id":"239851","messageId":"CABPQNSaC30p7TEOvc85u=+skjrFj17182vWWSL=QNVuvzVFE=w@mail.gmail.com","threadId":"36516","inReplyTo":"20140428083931.GA10257@camelia.ucw.cz","subject":"Re: [PATCH] Sleep 1 millisecond in poll() to avoid busy wait","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-04-28T09:07:24Z","receivedAt":"2014-04-28T09:07:24Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Apr 28, 2014 at 10:39 AM, Stepan Kasal <kasal@ucw.cz> wrote:\n> From: theoleblond <theodore.leblond@gmail.com>\n> Date: Wed, 16 May 2012 06:52:49 -0700\n>\n> I played around with this quite a bit. After trying some more complex\n> schemes, I found that what worked best is to just sleep 1 millisecond\n> between iterations. Though it's a very short time, it still completely\n> eliminates the busy wait condition, without hurting perf.\n>\n> There code uses SleepEx(1, TRUE) to sleep. See this page for a good\n> discussion of why that is better than calling SwitchToThread, which\n> is what was used previously:\n> http://stackoverflow.com/questions/1383943/switchtothread-vs-sleep1\n>\n> Note that calling SleepEx(0, TRUE) does *not* solve the busy wait.\n>\n> The most striking case was when testing on a UNC share with a large repo,\n> on a single CPU machine. Without the fix, it took 4 minutes 15 seconds,\n> and with the fix it took just 1:08! I think it's because git-upload-pack's\n> busy wait was eating the CPU away from the git process that's doing the\n> real work. With multi-proc, the timing is not much different, but tons of\n> CPU time is still wasted, which can be a killer on a server that needs to\n> do bunch of other things.\n>\n> I also tested the very fast local case, and didn't see any measurable\n> difference. On a big repo with 4500 files, the upload-pack took about 2\n> seconds with and without the fix.\n> ---\n>\n> This is one of the patches that lives in msysGit, could it be\n> accepted upstream?\n> It modifies the Windows compat function only.\n\ncompat/poll/poll.c comes from Gnulib, so it would be better to submit\nthe patch there and update.\n"},{"id":"239897","messageId":"20140428113815.GA10559@camelia.ucw.cz","threadId":"36516","inReplyTo":"CABPQNSaC30p7TEOvc85u=+skjrFj17182vWWSL=QNVuvzVFE=w@mail.gmail.com","subject":"Re: [PATCH] Sleep 1 millisecond in poll() to avoid busy wait","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-04-28T11:38:15Z","receivedAt":"2014-04-28T11:38:15Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"Hello,\n\nOn Mon, Apr 28, 2014 at 11:07:24AM +0200, Erik Faye-Lund wrote:\n> compat/poll/poll.c comes from Gnulib, so it would be better to submit\n> the patch there and update.\n\nwell, the change is in gnulib since 2012-05-21.\nBut the two versions has diverged a lot.\nCould you please just accept a backport of this one patch?\nI would be glad if msysgit could drop this patch.\n\nUpdated patch follows.\n\nStepan\n"},{"id":"239898","messageId":"20140428114224.GA11186@camelia.ucw.cz","threadId":"36516","inReplyTo":"20140428113815.GA10559@camelia.ucw.cz","subject":"[PATCH] poll/select: prevent busy-waiting","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-04-28T11:42:24Z","receivedAt":"2014-04-28T11:42:24Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"From: Paolo Bonzini <bonzini@gnu.org>\nDate: Mon, 21 May 2012 09:52:42 +0200\n\nBackported from Gnulib.\n\n2012-05-21  Paolo Bonzini  <bonzini@gnu.org>\n\n\tpoll/select: prevent busy-waiting.  SwitchToThread() only gives away\n\tthe rest of the current time slice to another thread in the current\n\tprocess. So if the thread that feeds the file decscriptor we're\n\tpolling is not in the current process, we get busy-waiting.\n\t* lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().\n\tPatch from Theodore Leblond.\n\t* lib/select.c: Split polling out of the loop that sets the output\n\tfd_sets.  Check for zero result and loop if the wait timeout is\n\tinfinite.\n\nSigned-off-by: Stepan Kasal <kasal@ucw.cz>\n---\n compat/poll/poll.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/poll/poll.c b/compat/poll/poll.c\nindex 31163f2..a9b41d8 100644\n--- a/compat/poll/poll.c\n+++ b/compat/poll/poll.c\n@@ -605,7 +605,7 @@ restart:\n \n   if (!rc && timeout == INFTIM)\n     {\n-      SwitchToThread();\n+      SleepEx (1, TRUE);\n       goto restart;\n     }\n \n-- \n1.9.2.msysgit.0.158.g6070cee\n"},{"id":"239899","messageId":"CABPQNSbDkE+Vff=4MmPO9oMfjRay6Oin51zZRoZ8mOEhGoaD3Q@mail.gmail.com","threadId":"36516","inReplyTo":"20140428114224.GA11186@camelia.ucw.cz","subject":"Re: [PATCH] poll/select: prevent busy-waiting","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-04-28T11:44:34Z","receivedAt":"2014-04-28T11:44:34Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Apr 28, 2014 at 1:42 PM, Stepan Kasal <kasal@ucw.cz> wrote:\n> From: Paolo Bonzini <bonzini@gnu.org>\n> Date: Mon, 21 May 2012 09:52:42 +0200\n>\n> Backported from Gnulib.\n>\n> 2012-05-21  Paolo Bonzini  <bonzini@gnu.org>\n>\n>         poll/select: prevent busy-waiting.  SwitchToThread() only gives away\n>         the rest of the current time slice to another thread in the current\n>         process. So if the thread that feeds the file decscriptor we're\n>         polling is not in the current process, we get busy-waiting.\n>         * lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().\n>         Patch from Theodore Leblond.\n>         * lib/select.c: Split polling out of the loop that sets the output\n>         fd_sets.  Check for zero result and loop if the wait timeout is\n>         infinite.\n>\n> Signed-off-by: Stepan Kasal <kasal@ucw.cz>\n> ---\n>  compat/poll/poll.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/compat/poll/poll.c b/compat/poll/poll.c\n> index 31163f2..a9b41d8 100644\n> --- a/compat/poll/poll.c\n> +++ b/compat/poll/poll.c\n> @@ -605,7 +605,7 @@ restart:\n>\n>    if (!rc && timeout == INFTIM)\n>      {\n> -      SwitchToThread();\n> +      SleepEx (1, TRUE);\n>        goto restart;\n>      }\n>\n> --\n> 1.9.2.msysgit.0.158.g6070cee\n>\n\nThanks for taking the effort!\n\nAcked-by: Erik Faye-Lund <kusmabite@gmail.com>\n"},{"id":"239938","messageId":"20140428142931.GA12056@camelia.ucw.cz","threadId":"36516","inReplyTo":"CABPQNSbDkE+Vff=4MmPO9oMfjRay6Oin51zZRoZ8mOEhGoaD3Q@mail.gmail.com","subject":"[PATCH] Windows: Always normalize paths to Windows-style","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-04-28T14:29:31Z","receivedAt":"2014-04-28T14:29:31Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"From: Heiko Voigt <hvoigt@hvoigt.net>\nDate: Thu, 22 Mar 2012 19:17:03 +0100\n\nIt appears that `pwd` returns the POSIX-style or the DOS-style path\ndepending which style the previous `cd` used. To normalize, enforce `pwd\n-W` in scripts.\n\n>From the original e-mail exchange:\n\nOn Thu, Mar 22, 2012 at 11:13:37AM +0100, Sebastian Schuberth wrote:\n> On Wed, Mar 21, 2012 at 22:21, Johannes Sixt <j6t@kdbg.org> wrote:\n>\n> > I build git and run its tests outside the msysgit environment. Does that\n> > explain the difference? (And I use CMD.)\n>\n> It does not make a difference for me. I started cmd.exe at\n> c:\\msysgit\\git\\t, added c:\\msysgit\\bin temporarily to PATH, and ran\n> \"sh t5526-fetch-submodules.sh -i -v\", and the test still fails.\n\nYes it probably does. Johannes said that he runs the tests outside of\nthe msysgit folder. That way there is only one path the submodule script\ngets reported and not two like '/c/msysgit/git' and '/git'.\n\nThat would explain to me why it is passing.\n\nI am afraid that the only solution is to patch msys itself to report the\nlong absolute path when passing window style paths to cd. Currently when\nI do\n\n\tcd c:/msysgit/git\n\nI will end up in '/git' instead of the long path.\n\nI found that there is a -W option to pwd in msys bash which makes it\nalways return the real windows path. A normalization in that direction\nis unique and thus might be more robust. Have a look at the attached\npatch. With this at least t5526 passes. I was not able to run the whole\ntestsuite properly at the moment. I can have a look at that tomorrow.\n\nWhat do you think?\n\nCheers Heiko\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\nHello,\nthis is another patch that lives in msysGit for a long time.\nOriginally, it had two parts:\n(Cf https://github.com/msysgit/git/commit/64a8a03 )\n\n1) adding alias pwd='pwd -W' to git-sh-setup.sh\n   This one went upstream, though as a shell function.\n\n2) revert of commit 4dce7d9b by Johannes Sixt <j6t@kdbg.org>\nThis mingw-specific commit was created less than 3 weeks before\nit was reverted.  And it stayed reverted for two years.\n\nCould you please either accept this patch, or revert 4dce7d9b ?\n(Both alternatives are exactly the same.)\n\nHave a nice day,\n\tStepan Kasal\n\n git-submodule.sh | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 4a30087..247273e 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -285,9 +285,6 @@ module_clone()\n \t# resolve any symlinks that might be present in $PWD\n \ta=$(cd_to_toplevel && cd \"$gitdir\" && pwd)/\n \tb=$(cd_to_toplevel && cd \"$sm_path\" && pwd)/\n-\t# normalize Windows-style absolute paths to POSIX-style absolute paths\n-\tcase $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} ;; esac\n-\tcase $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} ;; esac\n \t# Remove all common leading directories after a sanity check\n \tif test \"${a#$b}\" != \"$a\" || test \"${b#$a}\" != \"$b\"; then\n \t\tdie \"$(eval_gettext \"Gitdir '\\$a' is part of the submodule path '\\$b' or vice versa\")\"\n-- \n1.9.2.msysgit.0.158.g6070cee\n"},{"id":"239948","messageId":"535E6E4B.6070308@viscovery.net","threadId":"36516","inReplyTo":"CABPQNSbDkE+Vff=4MmPO9oMfjRay6Oin51zZRoZ8mOEhGoaD3Q@mail.gmail.com","subject":"Re: [PATCH] poll/select: prevent busy-waiting","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-04-28T15:05:47Z","receivedAt":"2014-04-28T15:05:47Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 4/28/2014 13:44, schrieb Erik Faye-Lund:\n> On Mon, Apr 28, 2014 at 1:42 PM, Stepan Kasal <kasal@ucw.cz> wrote:\n>> From: Paolo Bonzini <bonzini@gnu.org>\n>> Date: Mon, 21 May 2012 09:52:42 +0200\n>>\n>> Backported from Gnulib.\n>>\n>> 2012-05-21  Paolo Bonzini  <bonzini@gnu.org>\n>>\n>>         poll/select: prevent busy-waiting.  SwitchToThread() only gives away\n>>         the rest of the current time slice to another thread in the current\n>>         process. So if the thread that feeds the file decscriptor we're\n>>         polling is not in the current process, we get busy-waiting.\n>>         * lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().\n>>         Patch from Theodore Leblond.\n>>         * lib/select.c: Split polling out of the loop that sets the output\n>>         fd_sets.  Check for zero result and loop if the wait timeout is\n>>         infinite.\n>>\n>> Signed-off-by: Stepan Kasal <kasal@ucw.cz>\n>> ---\n>>  compat/poll/poll.c | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/compat/poll/poll.c b/compat/poll/poll.c\n>> index 31163f2..a9b41d8 100644\n>> --- a/compat/poll/poll.c\n>> +++ b/compat/poll/poll.c\n>> @@ -605,7 +605,7 @@ restart:\n>>\n>>    if (!rc && timeout == INFTIM)\n>>      {\n>> -      SwitchToThread();\n>> +      SleepEx (1, TRUE);\n>>        goto restart;\n>>      }\n>>\n>> --\n>> 1.9.2.msysgit.0.158.g6070cee\n>>\n> \n> Thanks for taking the effort!\n> \n> Acked-by: Erik Faye-Lund <kusmabite@gmail.com>\n\nThe patch text has my ACK, too (I've been using it since 2 years now), but\nI very much prefer the commit message of the earlier post. GNU standards\nw.r.t. commit messages are simply sub-par. ;-)\n\n-- Hannes\n"},{"id":"239958","messageId":"20140428152610.GA12357@camelia.ucw.cz","threadId":"36516","inReplyTo":"20140428142931.GA12056@camelia.ucw.cz","subject":"Re: [PATCH] Windows: Always normalize paths to Windows-style","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-04-28T15:26:10Z","receivedAt":"2014-04-28T15:26:10Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"The parent of this mail should have started a new thread.\nSorry for the typo.\nStepan\n"},{"id":"239962","messageId":"20140428153527.GB12357@camelia.ucw.cz","threadId":"36516","inReplyTo":"535E6E4B.6070308@viscovery.net","subject":"[PATCH v2] Sleep 1 millisecond in poll() to avoid busy wait","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-04-28T15:35:27Z","receivedAt":"2014-04-28T15:35:27Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"From: theoleblond <theodore.leblond@gmail.com>\nDate: Wed, 16 May 2012 06:52:49 -0700\n\nSwitchToThread() only gives away the rest of the current time slice\nto another thread in the current process. So if the thread that feeds\nthe file decscriptor we're polling is not in the current process, we\nget busy-waiting.\n\nI played around with this quite a bit. After trying some more complex\nschemes, I found that what worked best is to just sleep 1 millisecond\nbetween iterations. Though it's a very short time, it still completely\neliminates the busy wait condition, without hurting perf.\n\nThere code uses SleepEx(1, TRUE) to sleep. See this page for a good\ndiscussion of why that is better than calling SwitchToThread, which\nis what was used previously:\nhttp://stackoverflow.com/questions/1383943/switchtothread-vs-sleep1\n\nNote that calling SleepEx(0, TRUE) does *not* solve the busy wait.\n\nThe most striking case was when testing on a UNC share with a large repo,\non a single CPU machine. Without the fix, it took 4 minutes 15 seconds,\nand with the fix it took just 1:08! I think it's because git-upload-pack's\nbusy wait was eating the CPU away from the git process that's doing the\nreal work. With multi-proc, the timing is not much different, but tons of\nCPU time is still wasted, which can be a killer on a server that needs to\ndo bunch of other things.\n\nI also tested the very fast local case, and didn't see any measurable\ndifference. On a big repo with 4500 files, the upload-pack took about 2\nseconds with and without the fix.\n---\n\nOn Mon, Apr 28, 2014 at 05:05:47PM +0200, Johannes Sixt wrote:\n> [...] but I very much prefer the commit message of the earlier post.\n\n... but Paolo had a nice short description of the issue there;\nI inserted that to the top of the earlier commit message.\n\nThe latter diff (without the comment), gets us closer to gnulib's poll.c.\n\nHave a nice day,\n\tStepan\n\n compat/poll/poll.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/poll/poll.c b/compat/poll/poll.c\nindex 31163f2..a9b41d8 100644\n--- a/compat/poll/poll.c\n+++ b/compat/poll/poll.c\n@@ -605,7 +605,7 @@ restart:\n \n   if (!rc && timeout == INFTIM)\n     {\n-      SwitchToThread();\n+      SleepEx (1, TRUE);\n       goto restart;\n     }\n \n-- \n1.9.2.msysgit.0.158.g6070cee\n"},{"id":"239963","messageId":"CABPQNSbpyFzg8A8gLp2nJZeTRSLSb0Qs8ytZDsJLmi6VyER71Q@mail.gmail.com","threadId":"36516","inReplyTo":"20140428153527.GB12357@camelia.ucw.cz","subject":"Re: [PATCH v2] Sleep 1 millisecond in poll() to avoid busy wait","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-04-28T15:37:37Z","receivedAt":"2014-04-28T15:37:37Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Apr 28, 2014 at 5:35 PM, Stepan Kasal <kasal@ucw.cz> wrote:\n> From: theoleblond <theodore.leblond@gmail.com>\n> Date: Wed, 16 May 2012 06:52:49 -0700\n>\n> SwitchToThread() only gives away the rest of the current time slice\n> to another thread in the current process. So if the thread that feeds\n> the file decscriptor we're polling is not in the current process, we\n> get busy-waiting.\n>\n> I played around with this quite a bit. After trying some more complex\n> schemes, I found that what worked best is to just sleep 1 millisecond\n> between iterations. Though it's a very short time, it still completely\n> eliminates the busy wait condition, without hurting perf.\n>\n> There code uses SleepEx(1, TRUE) to sleep. See this page for a good\n> discussion of why that is better than calling SwitchToThread, which\n> is what was used previously:\n> http://stackoverflow.com/questions/1383943/switchtothread-vs-sleep1\n>\n> Note that calling SleepEx(0, TRUE) does *not* solve the busy wait.\n>\n> The most striking case was when testing on a UNC share with a large repo,\n> on a single CPU machine. Without the fix, it took 4 minutes 15 seconds,\n> and with the fix it took just 1:08! I think it's because git-upload-pack's\n> busy wait was eating the CPU away from the git process that's doing the\n> real work. With multi-proc, the timing is not much different, but tons of\n> CPU time is still wasted, which can be a killer on a server that needs to\n> do bunch of other things.\n>\n> I also tested the very fast local case, and didn't see any measurable\n> difference. On a big repo with 4500 files, the upload-pack took about 2\n> seconds with and without the fix.\n> ---\n>\n> On Mon, Apr 28, 2014 at 05:05:47PM +0200, Johannes Sixt wrote:\n>> [...] but I very much prefer the commit message of the earlier post.\n>\n> ... but Paolo had a nice short description of the issue there;\n> I inserted that to the top of the earlier commit message.\n>\n> The latter diff (without the comment), gets us closer to gnulib's poll.c.\n>\n\nGood stuff!\n"},{"id":"239990","messageId":"xmqqfvkxqo0l.fsf@gitster.dls.corp.google.com","threadId":"36516","inReplyTo":"CABPQNSbpyFzg8A8gLp2nJZeTRSLSb0Qs8ytZDsJLmi6VyER71Q@mail.gmail.com","subject":"Re: [PATCH v2] Sleep 1 millisecond in poll() to avoid busy wait","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-28T18:58:50Z","receivedAt":"2014-04-28T18:58:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> On Mon, Apr 28, 2014 at 5:35 PM, Stepan Kasal <kasal@ucw.cz> wrote:\n>> From: theoleblond <theodore.leblond@gmail.com>\n>> Date: Wed, 16 May 2012 06:52:49 -0700\n>>\n>> ...\n>> I also tested the very fast local case, and didn't see any measurable\n>> difference. On a big repo with 4500 files, the upload-pack took about 2\n>> seconds with and without the fix.\n>> ---\n>>\n> Good stuff!\n\nDo I expect to be forwarded this patch with proper provenance\nsign-off chain thru you?\n\nI do not mind adding Ack from you and J6t I saw in this thread\nmyself, perhaps like the following:\n\n    Subject: compat/poll: sleep 1 millisecond to avoid busy wait\n    From: Theodore Leblond <theodore.leblond@gmail.com>\n\n    SwitchToTrhead() only gives...\n    ...\n    seconds with and without the fix.\n\n    Signed-off-by: Theodore Leblond <theodore.leblond@gmail.com>\n    Signed-off-by: Stepan Kasal <kasal@ucw.cz>\n    Acked-by: Johannes Sixt <j6t@kdbg.org>\n    Acked-by: Erik Faye-Lund <kusmabite@gmail.com>\n\nI am just having this feeling that I lack the background on the\nevolution of this patch to say the above properly records the\nprovenance.\n\nOh, also in the last paragraph, who is \"I\" who tested?  Theodore, or\nStepan?\n\nThanks.\n    \n"},{"id":"240112","messageId":"20140429034727.GB15181@camelia.ucw.cz","threadId":"36516","inReplyTo":"xmqqfvkxqo0l.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] Sleep 1 millisecond in poll() to avoid busy wait","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-04-29T03:47:27Z","receivedAt":"2014-04-29T03:47:27Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"Hello Junio,\n\nthank you for pointing out the problems.\n\nLet me explain the background:\n\nAfter some discussion a one line fix to win32/poll.c was accepted to msysgit/git\nat 2012-05-16 https://github.com/msysgit/git/pull/7\n\nThe description of the commit looked like this:\n> I played around with this [...]\n> [...]\n> I also tested the very fast local case, and didn't see any measurable\n> difference. On a big repo with 4500 files, the upload-pack took about 2\n> seconds with and without the fix.\n... but there was no sign-off by Theodore.\n\nBecause poll.c comes from Gnulib, it was reported there as well.\nPaolo Bonzini accepted the fix for poll.c and added a fix for select.\nThe combined commit got this changelog entry:\n\n> 2012-05-21  Paolo Bonzini  <bonzini@gnu.org>\n> \n>         poll/select: prevent busy-waiting.  SwitchToThread() only gives away\n>         the rest of the current time slice to another thread in the current\n>         process. So if the thread that feeds the file decscriptor we're\n>         polling is not in the current process, we get busy-waiting.\n>         * lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().\n>         Patch from Theodore Leblond.\n>         * lib/select.c: Split polling out of the loop that sets the output\n>         fd_sets.  Check for zero result and loop if the wait timeout is\n>         infinite.\n\nThe patch that I (Stepan) submitted as \"v2\" combines things like this:\n- subject by Theodore\n- first paragraph by Paolo, concise problem description\n- rest from Theodore's original commit (\"I\" = Theodore, I suppose)\n- diff exctly as in gnulib commit\n\nOn Mon, Apr 28, 2014 at 11:58:50AM -0700, Junio C Hamano wrote:\n>     Subject: compat/poll: sleep 1 millisecond to avoid busy wait\n\nThanks for improving it.\n\n>     Signed-off-by: Theodore Leblond <theodore.leblond@gmail.com>\n>     Signed-off-by: Stepan Kasal <kasal@ucw.cz>\n>     Acked-by: Johannes Sixt <j6t@kdbg.org>\n>     Acked-by: Erik Faye-Lund <kusmabite@gmail.com>\n\nSorry that I forgot to add my sign-off (Stepan).\nBut I'm afraid I cannot add Theodore's, it was not in the original\ncommit.  But it is one line change; the real work was the analysis.\n\nStepan\n"},{"id":"240182","messageId":"xmqq7g68m641.fsf@gitster.dls.corp.google.com","threadId":"36516","inReplyTo":"20140429034727.GB15181@camelia.ucw.cz","subject":"Re: [PATCH v2] Sleep 1 millisecond in poll() to avoid busy wait","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-29T16:51:26Z","receivedAt":"2014-04-29T16:51:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stepan Kasal <kasal@ucw.cz> writes:\n\n> Hello Junio,\n>\n> thank you for pointing out the problems.\n>\n> Let me explain the background:\n>\n> After some discussion a one line fix to win32/poll.c was accepted to msysgit/git\n> at 2012-05-16 https://github.com/msysgit/git/pull/7\n>\n> The description of the commit looked like this:\n>> I played around with this [...]\n>> [...]\n>> I also tested the very fast local case, and didn't see any measurable\n>> difference. On a big repo with 4500 files, the upload-pack took about 2\n>> seconds with and without the fix.\n> ... but there was no sign-off by Theodore.\n>\n> Because poll.c comes from Gnulib, it was reported there as well.\n> Paolo Bonzini accepted the fix for poll.c and added a fix for select.\n> The combined commit got this changelog entry:\n>\n>> 2012-05-21  Paolo Bonzini  <bonzini@gnu.org>\n>> \n>>         poll/select: prevent busy-waiting.  SwitchToThread() only gives away\n>>         the rest of the current time slice to another thread in the current\n>>         process. So if the thread that feeds the file decscriptor we're\n>>         polling is not in the current process, we get busy-waiting.\n>>         * lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().\n>>         Patch from Theodore Leblond.\n>>         * lib/select.c: Split polling out of the loop that sets the output\n>>         fd_sets.  Check for zero result and loop if the wait timeout is\n>>         infinite.\n>\n> The patch that I (Stepan) submitted as \"v2\" combines things like this:\n> - subject by Theodore\n> - first paragraph by Paolo, concise problem description\n> - rest from Theodore's original commit (\"I\" = Theodore, I suppose)\n> - diff exctly as in gnulib commit\n>\n> On Mon, Apr 28, 2014 at 11:58:50AM -0700, Junio C Hamano wrote:\n>>     Subject: compat/poll: sleep 1 millisecond to avoid busy wait\n>\n> Thanks for improving it.\n>\n>>     Signed-off-by: Theodore Leblond <theodore.leblond@gmail.com>\n>>     Signed-off-by: Stepan Kasal <kasal@ucw.cz>\n>>     Acked-by: Johannes Sixt <j6t@kdbg.org>\n>>     Acked-by: Erik Faye-Lund <kusmabite@gmail.com>\n>\n> Sorry that I forgot to add my sign-off (Stepan).\n> But I'm afraid I cannot add Theodore's, it was not in the original\n> commit.  But it is one line change; the real work was the analysis.\n\nWell, the original git/pull/7 in msysgit repository says that is a\npatch by Theodore, so the kosher thing to do is to ask him (and I\nsee he is on the Cc list), and also ask msysgit folks (and I see\nthey are on the Cc list as well) to be a bit more careful when\nresponding to pull requests, especially given that it is our mutual\nbenefit to make sure we keep the changes between our two trees to\nthe minimum by upstreaming.\n\nI'll queue without the forged sign-off by Theodore, as I think DCO\n(1.1) (c) read loosely would let me do so ;-)\n\nThanks.\n"},{"id":"240932","messageId":"20140507184444.GB4013@sandbox-ub","threadId":"36516","inReplyTo":"20140428142931.GA12056@camelia.ucw.cz","subject":"Re: [PATCH] Windows: Always normalize paths to Windows-style","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2014-05-07T18:44:44Z","receivedAt":"2014-05-07T18:44:44Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Mon, Apr 28, 2014 at 04:29:31PM +0200, Stepan Kasal wrote:\n> this is another patch that lives in msysGit for a long time.\n> Originally, it had two parts:\n> (Cf https://github.com/msysgit/git/commit/64a8a03 )\n> \n> 1) adding alias pwd='pwd -W' to git-sh-setup.sh\n>    This one went upstream, though as a shell function.\n> \n> 2) revert of commit 4dce7d9b by Johannes Sixt <j6t@kdbg.org>\n> This mingw-specific commit was created less than 3 weeks before\n> it was reverted.  And it stayed reverted for two years.\n> \n> Could you please either accept this patch, or revert 4dce7d9b ?\n> (Both alternatives are exactly the same.)\n\nSorry for the late reply.  To me reverting (or omitting at the next\nrebasing merge) this patch sound fine, as it seems to be superseeded by\nthe upstream change.\n\nAs I can see thats already done on master, so it seems to be all good.\n\nCheers Heiko\n"},{"id":"240950","messageId":"xmqqmwet8gre.fsf@gitster.dls.corp.google.com","threadId":"36516","inReplyTo":"20140507184444.GB4013@sandbox-ub","subject":"Re: [PATCH] Windows: Always normalize paths to Windows-style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T20:40:05Z","receivedAt":"2014-05-07T20:40:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt <hvoigt@hvoigt.net> writes:\n\n> On Mon, Apr 28, 2014 at 04:29:31PM +0200, Stepan Kasal wrote:\n>> this is another patch that lives in msysGit for a long time.\n>> Originally, it had two parts:\n>> (Cf https://github.com/msysgit/git/commit/64a8a03 )\n>> \n>> 1) adding alias pwd='pwd -W' to git-sh-setup.sh\n>>    This one went upstream, though as a shell function.\n>> \n>> 2) revert of commit 4dce7d9b by Johannes Sixt <j6t@kdbg.org>\n>> This mingw-specific commit was created less than 3 weeks before\n>> it was reverted.  And it stayed reverted for two years.\n>> \n>> Could you please either accept this patch, or revert 4dce7d9b ?\n>> (Both alternatives are exactly the same.)\n>\n> Sorry for the late reply.  To me reverting (or omitting at the next\n> rebasing merge) this patch sound fine, as it seems to be superseeded by\n> the upstream change.\n>\n> As I can see thats already done on master, so it seems to be all good.\n\nAre you guys talking about be39048a (git-sh-setup.sh: Add an pwd()\nfunction for MinGW, 2012-04-17) which has been in since v1.7.11?\n\nThe change introduced by 4dce7d9b (submodules: fix ambiguous\nabsolute paths under Windows, 2012-03-04) still exists, but your\n\"reverting this patch sound fine\" confuses me.\n"},{"id":"241003","messageId":"20140508101115.GC4511@camelia.ucw.cz","threadId":"36516","inReplyTo":"xmqqmwet8gre.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Windows: Always normalize paths to Windows-style","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-05-08T10:11:15Z","receivedAt":"2014-05-08T10:11:15Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"Hello,\n\nOn Wed, May 07, 2014 at 01:40:05PM -0700, Junio C Hamano wrote:\n> Heiko Voigt <hvoigt@hvoigt.net> writes:\n> > On Mon, Apr 28, 2014 at 04:29:31PM +0200, Stepan Kasal wrote:\n> >> this is another patch that lives in msysGit for a long time.\n> >> Originally, it had two parts:\n> >> (Cf https://github.com/msysgit/git/commit/64a8a03 )\n> >> \n> >> 1) adding alias pwd='pwd -W' to git-sh-setup.sh\n> >>    This one went upstream, though as a shell function.\n> >> \n> >> 2) revert of commit 4dce7d9b by Johannes Sixt <j6t@kdbg.org>\n> >> This mingw-specific commit was created less than 3 weeks before\n> >> it was reverted.  And it stayed reverted for two years.\n> >> \n> >> Could you please either accept this patch, or revert 4dce7d9b ?\n> >> (Both alternatives are exactly the same.)\n> >\n> > Sorry for the late reply.  To me reverting (or omitting at the next\n> > rebasing merge) this patch sound fine, as it seems to be superseeded by\n> > the upstream change.\n> >\n> > As I can see thats already done on master, so it seems to be all good.\n\nThank you, Junio, for asking.\n(I'm afraid my previous mail was unclear.)\n\n> Are you guys talking about be39048a (git-sh-setup.sh: Add an pwd()\n> function for MinGW, 2012-04-17) which has been in since v1.7.11?\n\nThis one is ok, keep it.  (msysGit has been using an alias instead of\nfunction for several years, but msysGit/master was synced recently.)\n\n> The change introduced by 4dce7d9b (submodules: fix ambiguous\n> absolute paths under Windows, 2012-03-04) still exists, but your\n> \"reverting this patch sound fine\" confuses me.\n\nThis one was accepted to git, but was reverted in msysGit almost\nimmediately by 64a8a03.  Things stayed that way for 2 years.\nSo it seems no one has ever actually used this code.\nConsequently, I propose to revert 4dce7d9b from git.\n\nStepan\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"241026","messageId":"xmqqd2fo2fmi.fsf@gitster.dls.corp.google.com","threadId":"36516","inReplyTo":"20140508101115.GC4511@camelia.ucw.cz","subject":"Re: [PATCH] Windows: Always normalize paths to Windows-style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-08T20:13:25Z","receivedAt":"2014-05-08T20:13:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stepan Kasal <kasal@ucw.cz> writes:\n\n> This one was accepted to git, but was reverted in msysGit almost\n> immediately by 64a8a03.  Things stayed that way for 2 years.\n> So it seems no one has ever actually used this code.\n> Consequently, I propose to revert 4dce7d9b from git.\n\nOK, let's do this.  I'd like you to take the authorship of it (and\ncorrect any mistake I may have made in the description) with your\nsign-off.\n\nThanks.\n\n-- >8 --\nFrom: Stepan Kasal <kasal@ucw.cz>\nSubject: [PATCH] Revert \"submodules: fix ambiguous absolute paths under Windows\"\n\nThis reverts commit 4dce7d9b408b2935b85721b54a2010eda7ec1be9,\nwhich was originally done to help Windows but was almost\nimmediately reverted in msysGit, and the codebase kept this\nunnecessary divergence for almost two years.\n\nNot-Signed-off-yet-by: Stepan Kasal <kasal@ucw.cz>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n git-submodule.sh | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 66f5f75..821e6d4 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -285,9 +285,6 @@ module_clone()\n \t# resolve any symlinks that might be present in $PWD\n \ta=$(cd_to_toplevel && cd \"$gitdir\" && pwd)/\n \tb=$(cd_to_toplevel && cd \"$sm_path\" && pwd)/\n-\t# normalize Windows-style absolute paths to POSIX-style absolute paths\n-\tcase $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} ;; esac\n-\tcase $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} ;; esac\n \t# Remove all common leading directories after a sanity check\n \tif test \"${a#$b}\" != \"$a\" || test \"${b#$a}\" != \"$b\"; then\n \t\tdie \"$(eval_gettext \"Gitdir '\\$a' is part of the submodule path '\\$b' or vice versa\")\"\n-- \n2.0.0-rc2-397-ga0fd1fc\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"241029","messageId":"20140508203657.GA6203@camelia.ucw.cz","threadId":"36516","inReplyTo":"xmqqd2fo2fmi.fsf@gitster.dls.corp.google.com","subject":"[PATCH] Revert \"submodules: fix ambiguous absolute paths under Windows\"","fromName":"Stepan Kasal","fromEmail":"kasal@ucw.cz","sentAt":"2014-05-08T20:36:57Z","receivedAt":"2014-05-08T20:36:57Z","isPatch":true,"sender":{"key":"kasal@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/1481596?v=4"},"body":"This reverts commit 4dce7d9b408b2935b85721b54a2010eda7ec1be9,\nwhich was originally done to help Windows but was almost\nimmediately reverted in msysGit, and the codebase kept this\nunnecessary divergence for almost two years.\n\nSigned-off-by: Stepan Kasal <kasal@ucw.cz>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\nOn Thu, May 08, 2014 at 01:13:25PM -0700, Junio C Hamano wrote:\n> OK, let's do this.  I'd like you to take the authorship of it (and\n\nok; thank you for the nice description, no need for any correction.\n\n git-submodule.sh | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 66f5f75..821e6d4 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -285,9 +285,6 @@ module_clone()\n \t# resolve any symlinks that might be present in $PWD\n \ta=$(cd_to_toplevel && cd \"$gitdir\" && pwd)/\n \tb=$(cd_to_toplevel && cd \"$sm_path\" && pwd)/\n-\t# normalize Windows-style absolute paths to POSIX-style absolute paths\n-\tcase $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} ;; esac\n-\tcase $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} ;; esac\n \t# Remove all common leading directories after a sanity check\n \tif test \"${a#$b}\" != \"$a\" || test \"${b#$a}\" != \"$b\"; then\n \t\tdie \"$(eval_gettext \"Gitdir '\\$a' is part of the submodule path '\\$b' or vice versa\")\"\n-- \n2.0.0-rc2-397-ga0fd1fc\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"}]}