This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author vstinner
Recipients benjamin.peterson, bkabrda, flavio, matteo, ncoghlan, pitrou, rkuska, vstinner
Date 2015-09-19.08:01:06
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1442649668.87.0.992101366794.issue25084@psf.upfronthosting.co.za>
In-reply-to
Content
About the new optional balanced parameter added downstream to Fedora, the patch is very small compared to timedlock.patch. It only changes the Python code, not the C code:

--- /home/haypo/prog/python/2.7/Lib/threading.py	2014-11-05 15:05:11.432003853 +0100
+++ /usr/lib64/python2.7/threading.py	2015-07-05 16:16:00.000000000 +0200
@@ -306,7 +306,7 @@
         else:
             return True
 
-    def wait(self, timeout=None):
+    def wait(self, timeout=None, balancing=True):
         """Wait until notified or until a timeout occurs.
 
         If the calling thread has not acquired the lock when this method is
@@ -355,7 +355,10 @@
                     remaining = endtime - _time()
                     if remaining <= 0:
                         break
-                    delay = min(delay * 2, remaining, .05)
+                    if balancing:
+                        delay = min(delay * 2, remaining, 0.05)
+                    else:
+                        delay = remaining
                     _sleep(delay)
                 if not gotit:
                     if __debug__:

(with some additional changes to pass balanced parameter)


Compare:

$ diffstat balanced.patch 
 threading.py |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)


$ diffstat timedlock.patch 
 Doc/library/thread.rst      |   31 ++++++++++---
 Doc/library/threading.rst   |   32 +++++++++++++-
 Include/pythread.h          |   37 ++++++++++++++++
 Lib/dummy_thread.py         |    8 +++
 Lib/multiprocessing/pool.py |    6 +-
 Lib/test/lock_tests.py      |   44 +++++++++++++++++--
 Lib/threading.py            |   25 +++--------
 Modules/threadmodule.c      |   53 +++++++++++++++++++----
 Python/thread_nt.h          |   33 +++++++++++---
 Python/thread_pthread.h     |   98 +++++++++++++++++++++++++++++++++++---------
 10 files changed, 296 insertions(+), 71 deletions(-)

Do you know if the balanced parameter is used in the wild? Such code only works with Python 2 on Fedora/CentOS/RHEL, right?


"For upstream, I think it would be desirable to backport the timeout argument and simply have it throw RuntimeError if used with any of the backends other than thread_nt and thread_pthread."

It's not how Python handles portability. There are two choices in Python:

* some functions are only available on some platforms, ex: os.fork()
* the feature is supported by all platforms and Python uses a different implementation depending on the platform (ex: os.kill)

Raising RuntimeError depending on the platform is not a good practice.


"That way single source Python 2/3 code gains access to the timeout option, while code relying on one of the no-longer-available-in-Python-3 threading backends doesn't incur any stability risks."

This issue is about the performance of Condition.wait(). The timeout parameter already exists on Python 2 in Condition.wait(). So it's already possible to write a single code base compatible with Python 2 and Python 3.

If you are talking about adding e *new* timeout parameter to threading.Lock, it's a new feature. Usually, we don't add new features in minor releases. It makes maintenace more complex: you cannot guarantee anymore that an application written for Python 2.7.x will work on Python 2.7.y. If you really want this, I consider that a PEP is required to define exactly the scope of such change.


"However, I don't feel strongly enough about that to argue in favour of it against opposition - I'm happy enough to confine my argument to changing the downstream Fedora/RHEL/CentOS patch."

For this specific change (optimizing Condition.wait(timeout) using OS timeout), it makes sense to have a downstream only patch because the change will be specific to Linux.

Well, I'm not strongly opposed to optimize Python 2, but since the patch is not portable, if you really want it, I consider that the topic should be discussed on python-dev to have a wide audience.
History
Date User Action Args
2015-09-19 08:01:08vstinnersetrecipients: + vstinner, ncoghlan, pitrou, benjamin.peterson, bkabrda, rkuska, flavio, matteo
2015-09-19 08:01:08vstinnersetmessageid: <1442649668.87.0.992101366794.issue25084@psf.upfronthosting.co.za>
2015-09-19 08:01:08vstinnerlinkissue25084 messages
2015-09-19 08:01:07vstinnercreate