Re: [PATCH] Sleep 1 millisecond in poll() to avoid busy wait

2014-04-28 Thread Stepan Kasal
Hello,

On Mon, Apr 28, 2014 at 11:07:24AM +0200, Erik Faye-Lund wrote:
> compat/poll/poll.c comes from Gnulib, so it would be better to submit
> the patch there and update.

well, the change is in gnulib since 2012-05-21.
But the two versions has diverged a lot.
Could you please just accept a backport of this one patch?
I would be glad if msysgit could drop this patch.

Updated patch follows.

Stepan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Sleep 1 millisecond in poll() to avoid busy wait

2014-04-28 Thread Erik Faye-Lund
On Mon, Apr 28, 2014 at 10:39 AM, Stepan Kasal  wrote:
> From: theoleblond 
> Date: Wed, 16 May 2012 06:52:49 -0700
>
> I played around with this quite a bit. After trying some more complex
> schemes, I found that what worked best is to just sleep 1 millisecond
> between iterations. Though it's a very short time, it still completely
> eliminates the busy wait condition, without hurting perf.
>
> There code uses SleepEx(1, TRUE) to sleep. See this page for a good
> discussion of why that is better than calling SwitchToThread, which
> is what was used previously:
> http://stackoverflow.com/questions/1383943/switchtothread-vs-sleep1
>
> Note that calling SleepEx(0, TRUE) does *not* solve the busy wait.
>
> The most striking case was when testing on a UNC share with a large repo,
> on a single CPU machine. Without the fix, it took 4 minutes 15 seconds,
> and with the fix it took just 1:08! I think it's because git-upload-pack's
> busy wait was eating the CPU away from the git process that's doing the
> real work. With multi-proc, the timing is not much different, but tons of
> CPU time is still wasted, which can be a killer on a server that needs to
> do bunch of other things.
>
> I also tested the very fast local case, and didn't see any measurable
> difference. On a big repo with 4500 files, the upload-pack took about 2
> seconds with and without the fix.
> ---
>
> This is one of the patches that lives in msysGit, could it be
> accepted upstream?
> It modifies the Windows compat function only.

compat/poll/poll.c comes from Gnulib, so it would be better to submit
the patch there and update.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Sleep 1 millisecond in poll() to avoid busy wait

2014-04-28 Thread Stepan Kasal
From: theoleblond 
Date: Wed, 16 May 2012 06:52:49 -0700

I played around with this quite a bit. After trying some more complex
schemes, I found that what worked best is to just sleep 1 millisecond
between iterations. Though it's a very short time, it still completely
eliminates the busy wait condition, without hurting perf.

There code uses SleepEx(1, TRUE) to sleep. See this page for a good
discussion of why that is better than calling SwitchToThread, which
is what was used previously:
http://stackoverflow.com/questions/1383943/switchtothread-vs-sleep1

Note that calling SleepEx(0, TRUE) does *not* solve the busy wait.

The most striking case was when testing on a UNC share with a large repo,
on a single CPU machine. Without the fix, it took 4 minutes 15 seconds,
and with the fix it took just 1:08! I think it's because git-upload-pack's
busy wait was eating the CPU away from the git process that's doing the
real work. With multi-proc, the timing is not much different, but tons of
CPU time is still wasted, which can be a killer on a server that needs to
do bunch of other things.

I also tested the very fast local case, and didn't see any measurable
difference. On a big repo with 4500 files, the upload-pack took about 2
seconds with and without the fix.
---

This is one of the patches that lives in msysGit, could it be
accepted upstream?
It modifies the Windows compat function only.

Have a nice day,
Stepan

 compat/poll/poll.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/compat/poll/poll.c b/compat/poll/poll.c
index 31163f2..c5a9a93 100644
--- a/compat/poll/poll.c
+++ b/compat/poll/poll.c
@@ -605,7 +605,9 @@ restart:
 
   if (!rc && timeout == INFTIM)
 {
-  SwitchToThread();
+  /* Sleep 1 millisecond to avoid busy wait */
+  SleepEx(1, TRUE);
+
   goto restart;
 }
 
-- 
1.9.2.msysgit.0.158.g6070cee

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html