[issue15038] Optimize python Locks on Windows

Changes by Kristján Valur Jónsson : -- resolution: -> fixed status: open -> closed ___ Python tracker ___ ___ Python-bugs-list mailin

[issue15038] Optimize python Locks on Windows

Roundup Robot added the comment: New changeset 08b87dda6f6a by Kristján Valur Jónsson in branch 'default': Issue #15038 : Fixing the condition broadcast and docs. http://hg.python.org/cpython/rev/08b87dda6f6a New changeset fde60d3f542e by Kristján Valur Jónsson in branch '3.3': Issue #15038 : Fi

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: Thanks, Benjamin. That's what reviews are for :) -- ___ Python tracker ___ ___ Python-bugs

[issue15038] Optimize python Locks on Windows

Benjamin Peterson added the comment: I see dead code here: Py_LOCAL_INLINE(int) PyCOND_BROADCAST(PyCOND_T *cv) { if (cv->waiting > 0) { return ReleaseSemaphore(cv->sem, cv->waiting, NULL) ? 0 : -1; cv->waiting = 0; } return 0; } -- nosy: +benjamin.pe

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: Thanks. The comments remain as the last bit of the original Condition Variable emulation code that was put in place for the new GIL. -- ___ Python tracker ___

[issue15038] Optimize python Locks on Windows

Richard Oudkerk added the comment: 1.41 Generic emulations of the pthread_cond_* API using 1.42 earlier Win32 functions can be found on the Web. 1.43 The following read can be edificating (or not): 1.44 http://www.cse.wustl.edu/~schmidt/win32-cv-1.html 1.45 +

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: Ah, right, the lock x, I forgot about that. -- ___ Python tracker ___ ___ Python-bugs-list

[issue15038] Optimize python Locks on Windows

Richard Oudkerk added the comment: > It's an interesting article Richard, but I don't see how their 2nd attempt > solves the problem. All it does is block the thread doing the Signal(), > not other threads, from stealing the wakeup. Do you mean the listing on page 5? (The earlier attempts w

[issue15038] Optimize python Locks on Windows

Roundup Robot added the comment: New changeset d7a72fdcc168 by Kristjan Valur Jonsson in branch 'default': Issue #15038: Document caveats with the emulated condition variables. http://hg.python.org/cpython/rev/d7a72fdcc168 -- ___ Python tracker

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: Right. Without holding the mutex, the definition of "already blocked" is of course meaningless, since only the holding the mutex can define any ordering. pthread standard indeed says "however, if predictable scheduling behaviour is required, then that

[issue15038] Optimize python Locks on Windows

Richard Oudkerk added the comment: The notes should also mention that PyCOND_SIGNAL() and PyCOND_BROADCAST() must be called while holding the mutex. (pthreads does not have that restriction.) -- ___ Python tracker

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: It's an interesting article Richard, but I don't see how their 2nd attempt solves the probvlem. All it does is block the thread doing the Signal(), not other threads, from stealing the wakeup. I think I know how to fix this correctly, using a separat

[issue15038] Optimize python Locks on Windows

Antoine Pitrou added the comment: > > The implementation for multiprocessing.Condition is virtually the same > > as Listing 3 which the author says he thinks is "formally correct" but > > with "a fundamental performance problem". > > To me, it seems similar to the last listing (under "The Seque

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: The problem Richard describes isn´t a lost wakeup. PyCOND_SIGNAL _will_ wake up _at least_ one thread. It just isn't guaranteed to be one of those who previously called PyCOND_WAIT(): It could be a latecomer to the game, including the one who called

[issue15038] Optimize python Locks on Windows

Antoine Pitrou added the comment: > The problem Richard describes isn´t a lost wakeup. PyCOND_SIGNAL > _will_ wake up _at least_ one thread. It just isn't guaranteed to be > one of those who previously called PyCOND_WAIT(): It could be a > latecomer to the game, including the one who called Si

[issue15038] Optimize python Locks on Windows

Antoine Pitrou added the comment: > The implementation for multiprocessing.Condition is virtually the same > as Listing 3 which the author says he thinks is "formally correct" but > with "a fundamental performance problem". To me, it seems similar to the last listing (under "The Sequel—NT and P

[issue15038] Optimize python Locks on Windows

Richard Oudkerk added the comment: The implementation in condvar.h is basically the same as one of the attempts mentioned in http://birrell.org/andrew/papers/ImplementingCVs.pdf (Listing 2 fixed to use non-binary semaphores.) The implementation for multiprocessing.Condition is virtually

[issue15038] Optimize python Locks on Windows

Richard Oudkerk added the comment: > Let me elaborate: the GIL can perhaps suffer lost wakeups from time to > time. The Lock API certainly shouldn't. I think with FORCE_SWITCHING defined (the default?) it is not possible for the thread releasing the GIL to immediately reacquire it (unless the

[issue15038] Optimize python Locks on Windows

Antoine Pitrou added the comment: > However, if this lost wakeup problem can affect current users of the API, > then it sounds unacceptable. Let me elaborate: the GIL can perhaps suffer lost wakeups from time to time. The Lock API certainly shouldn't. --

[issue15038] Optimize python Locks on Windows

Antoine Pitrou added the comment: > Personally I'm not sure it is a wise guarantee to make. If you make sure internal users are immune to this issue, then fine (but make sure to document it somewhere). However, if this lost wakeup problem can affect current users of the API, then it sounds un

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: Yes, another correct observation. This can if a thread is interrupted in between releasing the mutex and waiting for the semaphore. I see no way around this apart from manually creating event objects for every thread. Personally I'm not sure it is a

[issue15038] Optimize python Locks on Windows

Richard Oudkerk added the comment: Standard condition variables have the following guarantees: * if there are any waiters then signal()/notify() will awaken at least one of them; * if there are any waiters then broadcast()/notify_all() will awaken all of them. The implementation in condvar.h

[issue15038] Optimize python Locks on Windows

Roundup Robot added the comment: New changeset 110b38c36a31 by Kristjan Valur Jonsson in branch 'default': Issue #15038: http://hg.python.org/cpython/rev/110b38c36a31 -- ___ Python tracker

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: Well spotted. This probably fixes the failure we saw in the buildbots as well. -- ___ Python tracker ___ _

[issue15038] Optimize python Locks on Windows

Richard Oudkerk added the comment: The old version was 243 __inline static void _cond_signal(COND_T *cond) { 244 /* NOTE: This must be called with the mutex held */ 245 if (cond->n_waiting > 0) { 246 if (!ReleaseSemaphore(cond->sem, 1, NULL)) 247 Py_

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: You are right, Richard. Thanks for pointing this out. This is not a new problem, however, because this code has been in the New GIL since it was launched. The purpose of the "n_waiting" member is to make "signal" a no-op when no one is there waitin

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: Thanks Antoine. I tested this in my virtualbox so something new must have happened... Anyway, the GIL code should not have changed from before, only moved about slightly. I´ll figure out what happened -- ___

[issue15038] Optimize python Locks on Windows

Richard Oudkerk added the comment: Py_LOCAL_INLINE(int) _PyCOND_WAIT_MS(PyCOND_T *cv, PyMUTEX_T *cs, DWORD ms) { DWORD wait; cv->waiting++; PyMUTEX_UNLOCK(cs); /* "lost wakeup bug" would occur if the caller were interrupted here, * but we are safe because we are using a sema

[issue15038] Optimize python Locks on Windows

Antoine Pitrou added the comment: There's a problem here: Fatal Python error: PyCOND_SIGNAL(gil_cond) failed http://www.python.org/dev/buildbot/all/builders/x86%20XP-4%203.x/builds/6859/steps/test/logs/stdio -- ___ Python tracker

[issue15038] Optimize python Locks on Windows

Roundup Robot added the comment: New changeset 978326f98316 by Kristján Valur Jónsson in branch 'default': Issue #15038: Optimize python Locks on Windows http://hg.python.org/cpython/rev/978326f98316 -- nosy: +python-dev ___ Python tracker

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: Ok, I take the lack of negative reviews as a general approvement. I'll improve comments a bit, write the appropriate NEWS item and make a commit soon. -- ___ Python tracker

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: While I'm confident about the correctness of this implementation (it´s in production use right now) I´d like comments on the architecture. - Are people comfortable with the notion of an include file with an inline implementation of portable condition

[issue15038] Optimize python Locks on Windows

Changes by Antoine Pitrou : -- stage: -> patch review type: enhancement -> performance ___ Python tracker ___ ___ Python-bugs-list ma

[issue15038] Optimize python Locks on Windows

Changes by Antoine Pitrou : -- superseder: Locks broken wrt timeouts on Windows -> ___ Python tracker ___ ___ Python-bugs-list mailin

[issue15038] Optimize python Locks on Windows

Changes by STINNER Victor : -- nosy: +haypo ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: I've tested Ubuntu 64 myself using a Virtualbox, confirming that the pythread functionality is untouched. (funny how those vi keystrokes seem to be embedded into your amygdala after decades of disuse) -- __

[issue15038] Optimize python Locks on Windows

Paul Moore added the comment: Applies and builds cleanly on Win7 32-bit. The speed difference is visible here too: PS D:\Data\cpython\PCbuild> .\python.exe -m timeit -s "from _thread import allocate_lock; l=allocate_lock()" "l.acquire();l.release()" 100 loops, best of 3: 0.608 usec per lo

[issue15038] Optimize python Locks on Windows

Kristján Valur Jónsson added the comment: This defect springs out of issue #11618 -- superseder: -> Locks broken wrt timeouts on Windows ___ Python tracker ___

[issue15038] Optimize python Locks on Windows

New submission from Kristján Valur Jónsson : The attached patch does three things: " - Abstract the condition variable used by ceval_gil.h into a separate file, condvar.h. It now defines a PyMUTEX_T, PyCOND_T and associated functions. This file can be used by different parts of the python core.