Re: ntdll: make NtDelayExecution a bit more efficient

[Alexandre Julliard]

Graham <graham.knap(a)gmail.com> writes:
> This patch is vaguely related to bug 24558. It eliminates a few
> syscalls in NtDelayExecution:
>
> 1. If the caller requests a zero-wait yield, then do just that, and
> nothing more.

That's what the existing code already does.
> 2. 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 Julliard
julliard(a)winehq.org

ntdll: make NtDelayExecution a bit more efficient

[Graham]

Alexandre wrote:
> That's what the existing code already does.

Indeed. I don't know what I was thinking...
> > 2. 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

ntdll: make NtDelayExecution a bit more efficient

[Michael Stefaniuc]

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...
> > > 2. 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

ntdll: make NtDelayExecution a bit more efficient

[Graham Knap]

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

ntdll: make NtDelayExecution a bit more efficient

[Stefan Dösinger]

-----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);

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJROfmEAAoJEN0/YqbEcdMwetYP/3NB0CEhVB0VtTa4jfxGOdx8
cMFguVUpxjVAHoV6FlovydW4Eb/ERGsjn8HFocabuyvLkMshzwt22AZC4REfaibZ
zAywbrdjbu0FQEURlhMuDw+jZvEoqNRmYBSWq5omxB+HuWu6YD+Hkw9zfl7AKdSa
lrCFilJ3dFscY5NTViZN75fAXh7HrYKUjXRiPUln5qUBFvKcSQniUXiaLheIgSRA
UqllLxuTAX0wKQkYrxGBiPcabkR3Rdn6JyIzWytmbUS3PCJYrWbPvRdV+/Rn4RjY
YGvrMzffJGQu6MlwZkxi8gr/tZc6p0AWkJ4EmHd6zYipJJrKYER9GVCsmrex15BR
4d6ebL9tB3jVboAb9rLNl1ZPHKUCB8no9+uxFYJv5+cFAspJstFzupW18z2LB5X+
DCzemYXlW3dWueFxNM+1PjE3vSAL+5lRkmhKEL+7OMPXC5jRP2XDWauVYHrSX1oF
ZmgfjU7/VNl4Li1grqLP8Ux0vv1RLPsNfar8ycByBh4dCpzv+ZL3MnkueSbryqBH
TEvSr4WIDe3YVDjxwNdbiSGUFEwgILfrZC4SLkR8NOVu2Nl26z1wsbE4vk2XTMGs
9EBMXsp5EdFnvK3etNphrWcJYJKA6P0GlFF/iXFzR2z3UKQBddQLw/LnFmaEprx2
20fKvxaGI1pIlGBi3Hl+
=p/ep
-----END PGP SIGNATURE-----

ntdll: make NtDelayExecution a bit more efficient

[Juan Lang]

On Fri, Mar 8, 2013 at 6:11 AM, Graham Knap <graham.knap(a)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

ntdll: make NtDelayExecution a bit more efficient

[Francois Gouget]

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(a)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(a)winehq.org>
Date:   Mon Oct 11 20:11:01 2004 +0000

    Implemented NtYieldExecution.

commit 8099c2b9fd5b80f35868936a6e96f732106c3286
Author: Jeremy White <jwhite(a)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 <fgouget(a)codeweavers.com>

ntdll: make NtDelayExecution a bit more efficient

[Graham Knap]

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