Graham graham.knap@gmail.com writes:
This patch is vaguely related to bug 24558. It eliminates a few syscalls in NtDelayExecution:
- If the caller requests a zero-wait yield, then do just that, and
nothing more.
That's what the existing code already does.
- If you're about to block on select(), then I don't see any point in
preceding that with a call to sched_yield().
This was added for a reason; most likely you'll have to write tests.
Alexandre wrote:
That's what the existing code already does.
Indeed. I don't know what I was thinking...
- If you're about to block on select(), then I don't see any point in
preceding that with a call to sched_yield().
This was added for a reason; most likely you'll have to write tests.
i.e. commit 8099c2b9. JW says "... to more closely resemble Windows behavior. The key is to yield in a Sleep..."
I haven't yet figured out how he came up with this analysis, but I think it's safe to assume that he is correct, and my patch is garbage. Lesson learned: consult history.
-- graham
On 03/06/2013 07:10 PM, Graham wrote:
Alexandre wrote:
That's what the existing code already does.
Indeed. I don't know what I was thinking...
- If you're about to block on select(), then I don't see any point in
preceding that with a call to sched_yield().
This was added for a reason; most likely you'll have to write tests.
i.e. commit 8099c2b9. JW says "... to more closely resemble Windows behavior. The key is to yield in a Sleep..."
JW is Jeremy White so us old timers chuckle now ;)
I haven't yet figured out how he came up with this analysis, but I think it's safe to assume that he is correct, and my patch is garbage. Lesson learned: consult history.
"Date: Tue Nov 2 19:32:03 2004 +0000" and the comment mentions Win2K. If the ancient wisdom isn't backed by tests there's a fair chance that it might not be applicable today. Or that it was a wrong theory back then too.
bye michael
Michael Stefaniuc wrote:
i.e. commit 8099c2b9. JW says "... to more closely resemble Windows behavior. The key is to yield in a Sleep..."
JW is Jeremy White so us old timers chuckle now ;)
I know the name, but nothing more. Why is this funny?
If the ancient wisdom isn't backed by tests there's a fair chance that it might not be applicable today. Or that it was a wrong theory
Interesting. Unfortunately, I have no idea how to go about writing tests for this. Can anyone offer any suggestions?
-- graham
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2013-03-08 15:11, schrieb Graham Knap:
Michael Stefaniuc wrote:
If the ancient wisdom isn't backed by tests there's a fair chance that it might not be applicable today. Or that it was a wrong theory
This is guesswork, but maybe the yield is there to make code like this work (or more likely to work):
start = GetTickCount(); Sleep(0); end = GetTickCount(); output = input / (end - start);
On Fri, Mar 8, 2013 at 6:11 AM, Graham Knap graham.knap@gmail.com wrote:
Michael Stefaniuc wrote:
i.e. commit 8099c2b9. JW says "... to more closely resemble Windows behavior. The key is to yield in a Sleep..."
JW is Jeremy White so us old timers chuckle now ;)
I know the name, but nothing more. Why is this funny?
He's the CEO of Codeweavers, and he's the first one to admit he doesn't write great code, or at least that Alexandre rejects his code a lot :)
If the ancient wisdom isn't backed by tests there's a fair chance that
it might not be applicable today. Or that it was a wrong theory
Interesting. Unfortunately, I have no idea how to go about writing tests for this. Can anyone offer any suggestions?
It's, well, hard to test. MSDN suggests it was true in XP, but subsequently changed: http://msdn.microsoft.com/en-us/library/windows/desktop/ms686298(v=vs.85).as...
I no longer recall which app it was Jeremy was working on fixing, but there was at least one app that depended on this behavior. See Jeremy's posts on "RFC: yield on Waits" and "Threading issues". --Juan
On Fri, 8 Mar 2013, Juan Lang wrote: [...]
Michael Stefaniuc wrote:
i.e. commit 8099c2b9. JW says "... to more closely resemble Windows behavior. The key is to yield in a Sleep..."
I think it's really a three patch series and the first one has the clearer commit message:
commit 08c0f691cebc0994bb94bc160c080cccf8068561 Author: Jeremy White jwhite@codeweavers.com Date: Sat Oct 9 02:26:29 2004 +0000
Made NtDelayExecution with a 0 timeout yield the CPU, as it is supposed to.
commit de91a8dd0fd01da20656a4f6be423e5ae2df785f Author: Alexandre Julliard julliard@winehq.org Date: Mon Oct 11 20:11:01 2004 +0000
Implemented NtYieldExecution.
commit 8099c2b9fd5b80f35868936a6e96f732106c3286 Author: Jeremy White jwhite@codeweavers.com Date: Tue Nov 2 19:32:03 2004 +0000
Tune the behavior of Sleep() and Waitxxx() to more closely resemble Windows behavior. The key is to yield in a Sleep and in any Wait that times out.
If the ancient wisdom isn't backed by tests there's a fair chance that
it might not be applicable today. Or that it was a wrong theory
[...]
I no longer recall which app it was Jeremy was working on fixing, but there was at least one app that depended on this behavior. See Jeremy's posts on "RFC: yield on Waits" and "Threading issues".
http://www.winehq.org/pipermail/wine-devel/2004-November/030796.html
I suspect it was the Shockwave Director plugin(*). If I remember correctly it had a thread in a tight loop just calling PeekMessage() and Sleep(0) or some such. Then it had another thread that was doing the real work. But because the first thread was not yielding the second's performance suffered (even more so as the processors of the time where mostly single core).
(*) Nothing to do with Shockwave Flash, aka Macromedia Flash aka Adobe Flash.
Francois Gouget wrote:
I think it's really a three patch series and the first one has the clearer commit message:
Made NtDelayExecution with a 0 timeout yield the CPU, as it is supposed to.
I agree that this is the correct behaviour for the "zero timeout" case. The comments that I submitted with my patch were incorrect, because I misread the existing code. The patch that I submitted would not have changed this behaviour, though it did reorganize a few lines of code.
I had proposed to change the "nonzero timeout" case. If the caller specifies a non-zero timeout value, the existing code calls:
1 if the timeout is negative: NtQuerySystemTime 2 NtYieldExecution 3 begin loop: 3.1 NtQuerySystemTime 3.2 select
We break out of the loop when "select" has waited the specified time, and was not interrupted by a signal. (Right? Please tell me if I've misinterpreted the code again.)
I suggested changing it to:
1 NtQuerySystemTime 2 begin loop: 2.1 select 2.2 NtQuerySystemTime
To explain:
* I think there is no need to call sched_yield before blocking on select. (But I haven't yet come up with a way to prove it.)
* We no longer need to measure how long sched_yield made us wait, so we can reorder the NtQuerySystemTime calls. The break condition is unchanged, so the second NtQuerySystemTime call is often skipped.
http://www.winehq.org/pipermail/wine-devel/2004-November/030796.html
Thanks, I will look at this.
-- graham
-
Alexandre Julliard -
Francois Gouget -
Graham -
Graham Knap
-
Juan Lang -
Michael Stefaniuc -
Stefan Dösinger