From: theoleblond <redacted>
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(-)
From: Erik Faye-Lund <hidden> Date: 2016-06-15 23:00:51
On Mon, Apr 28, 2014 at 10:39 AM, Stepan Kasal [off-list ref] wrote:
From: theoleblond <redacted>
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.
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
From: Paolo Bonzini <redacted>
Date: Mon, 21 May 2012 09:52:42 +0200
Backported from Gnulib.
2012-05-21 Paolo Bonzini [off-list ref]
poll/select: prevent busy-waiting. SwitchToThread() only gives away
the rest of the current time slice to another thread in the current
process. So if the thread that feeds the file decscriptor we're
polling is not in the current process, we get busy-waiting.
* lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().
Patch from Theodore Leblond.
* lib/select.c: Split polling out of the loop that sets the output
fd_sets. Check for zero result and loop if the wait timeout is
infinite.
Signed-off-by: Stepan Kasal <redacted>
---
compat/poll/poll.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Erik Faye-Lund <hidden> Date: 2016-06-15 23:00:52
On Mon, Apr 28, 2014 at 1:42 PM, Stepan Kasal [off-list ref] wrote:
quoted hunk
From: Paolo Bonzini <redacted>
Date: Mon, 21 May 2012 09:52:42 +0200
Backported from Gnulib.
2012-05-21 Paolo Bonzini [off-list ref]
poll/select: prevent busy-waiting. SwitchToThread() only gives away
the rest of the current time slice to another thread in the current
process. So if the thread that feeds the file decscriptor we're
polling is not in the current process, we get busy-waiting.
* lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().
Patch from Theodore Leblond.
* lib/select.c: Split polling out of the loop that sets the output
fd_sets. Check for zero result and loop if the wait timeout is
infinite.
Signed-off-by: Stepan Kasal <redacted>
---
compat/poll/poll.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Heiko Voigt <redacted>
Date: Thu, 22 Mar 2012 19:17:03 +0100
It appears that `pwd` returns the POSIX-style or the DOS-style path
depending which style the previous `cd` used. To normalize, enforce `pwd
-W` in scripts.
From the original e-mail exchange:
On Thu, Mar 22, 2012 at 11:13:37AM +0100, Sebastian Schuberth wrote:
On Wed, Mar 21, 2012 at 22:21, Johannes Sixt [off-list ref] wrote:
quoted
I build git and run its tests outside the msysgit environment. Does that
explain the difference? (And I use CMD.)
It does not make a difference for me. I started cmd.exe at
c:\msysgit\git\t, added c:\msysgit\bin temporarily to PATH, and ran
"sh t5526-fetch-submodules.sh -i -v", and the test still fails.
Yes it probably does. Johannes said that he runs the tests outside of
the msysgit folder. That way there is only one path the submodule script
gets reported and not two like '/c/msysgit/git' and '/git'.
That would explain to me why it is passing.
I am afraid that the only solution is to patch msys itself to report the
long absolute path when passing window style paths to cd. Currently when
I do
cd c:/msysgit/git
I will end up in '/git' instead of the long path.
I found that there is a -W option to pwd in msys bash which makes it
always return the real windows path. A normalization in that direction
is unique and thus might be more robust. Have a look at the attached
patch. With this at least t5526 passes. I was not able to run the whole
testsuite properly at the moment. I can have a look at that tomorrow.
What do you think?
Cheers Heiko
Signed-off-by: Johannes Schindelin <redacted>
---
Hello,
this is another patch that lives in msysGit for a long time.
Originally, it had two parts:
(Cf https://github.com/msysgit/git/commit/64a8a03 )
1) adding alias pwd='pwd -W' to git-sh-setup.sh
This one went upstream, though as a shell function.
2) revert of commit 4dce7d9b by Johannes Sixt [off-list ref]
This mingw-specific commit was created less than 3 weeks before
it was reverted. And it stayed reverted for two years.
Could you please either accept this patch, or revert 4dce7d9b ?
(Both alternatives are exactly the same.)
Have a nice day,
Stepan Kasal
git-submodule.sh | 3 ---
1 file changed, 3 deletions(-)
@@ -285,9 +285,6 @@ module_clone()# resolve any symlinks that might be present in $PWDa=$(cd_to_toplevel&&cd"$gitdir"&&pwd)/b=$(cd_to_toplevel&&cd"$sm_path"&&pwd)/-# normalize Windows-style absolute paths to POSIX-style absolute paths-case$ain[a-zA-Z]:/*)a=/${a%%:*}${a#*:};;esac-case$bin[a-zA-Z]:/*)b=/${b%%:*}${b#*:};;esac# Remove all common leading directories after a sanity checkiftest"${a#$b}"!="$a"||test"${b#$a}"!="$b";thendie"$(eval_gettext"Gitdir '\$a' is part of the submodule path '\$b' or vice versa")"
From: Johannes Sixt <hidden> Date: 2016-06-15 23:00:52
Am 4/28/2014 13:44, schrieb Erik Faye-Lund:
On Mon, Apr 28, 2014 at 1:42 PM, Stepan Kasal [off-list ref] wrote:
quoted
From: Paolo Bonzini <redacted>
Date: Mon, 21 May 2012 09:52:42 +0200
Backported from Gnulib.
2012-05-21 Paolo Bonzini [off-list ref]
poll/select: prevent busy-waiting. SwitchToThread() only gives away
the rest of the current time slice to another thread in the current
process. So if the thread that feeds the file decscriptor we're
polling is not in the current process, we get busy-waiting.
* lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().
Patch from Theodore Leblond.
* lib/select.c: Split polling out of the loop that sets the output
fd_sets. Check for zero result and loop if the wait timeout is
infinite.
Signed-off-by: Stepan Kasal <redacted>
---
compat/poll/poll.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Thanks for taking the effort!
Acked-by: Erik Faye-Lund <redacted>
The patch text has my ACK, too (I've been using it since 2 years now), but
I very much prefer the commit message of the earlier post. GNU standards
w.r.t. commit messages are simply sub-par. ;-)
-- Hannes
From: theoleblond <redacted>
Date: Wed, 16 May 2012 06:52:49 -0700
SwitchToThread() only gives away the rest of the current time slice
to another thread in the current process. So if the thread that feeds
the file decscriptor we're polling is not in the current process, we
get busy-waiting.
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.
---
On Mon, Apr 28, 2014 at 05:05:47PM +0200, Johannes Sixt wrote:
[...] but I very much prefer the commit message of the earlier post.
... but Paolo had a nice short description of the issue there;
I inserted that to the top of the earlier commit message.
The latter diff (without the comment), gets us closer to gnulib's poll.c.
Have a nice day,
Stepan
compat/poll/poll.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Erik Faye-Lund <hidden> Date: 2016-06-15 23:00:53
On Mon, Apr 28, 2014 at 5:35 PM, Stepan Kasal [off-list ref] wrote:
From: theoleblond <redacted>
Date: Wed, 16 May 2012 06:52:49 -0700
SwitchToThread() only gives away the rest of the current time slice
to another thread in the current process. So if the thread that feeds
the file decscriptor we're polling is not in the current process, we
get busy-waiting.
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.
---
On Mon, Apr 28, 2014 at 05:05:47PM +0200, Johannes Sixt wrote:
quoted
[...] but I very much prefer the commit message of the earlier post.
... but Paolo had a nice short description of the issue there;
I inserted that to the top of the earlier commit message.
The latter diff (without the comment), gets us closer to gnulib's poll.c.
Hi,
On Mon, Apr 28, 2014 at 04:29:31PM +0200, Stepan Kasal wrote:
this is another patch that lives in msysGit for a long time.
Originally, it had two parts:
(Cf https://github.com/msysgit/git/commit/64a8a03 )
1) adding alias pwd='pwd -W' to git-sh-setup.sh
This one went upstream, though as a shell function.
2) revert of commit 4dce7d9b by Johannes Sixt [off-list ref]
This mingw-specific commit was created less than 3 weeks before
it was reverted. And it stayed reverted for two years.
Could you please either accept this patch, or revert 4dce7d9b ?
(Both alternatives are exactly the same.)
Sorry for the late reply. To me reverting (or omitting at the next
rebasing merge) this patch sound fine, as it seems to be superseeded by
the upstream change.
As I can see thats already done on master, so it seems to be all good.
Cheers Heiko