[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[STINNER Victor]

STINNER Victor added the comment:

Nick Coghlan added the comment:
> +1 - after the further discussion, addressing this downstream as a patch 
> specifically to the pthread backend sounds good to me.

Cool, I like when we agree :-)

@Flavio Grossi: Sorry for you, but it's time to upgrade to Python 3.
Twisted made great progress on Python 3 support. For Apache Thrift,
maybe you can help them to port the library to Python 3?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Nick Coghlan]

Nick Coghlan added the comment:

+1 - after the further discussion, addressing this downstream as a patch 
specifically to the pthread backend sounds good to me.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[STINNER Victor]

STINNER Victor added the comment:

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.py2014-11-05 
15:05:11.432003853 +0100
+++ /usr/lib64/python2.7/threading.py   2015-07-05 16:16:00.0 +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.

--
Added file: http://bugs.python.org/file40512/balanced.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Flavio Grossi]

Flavio Grossi added the comment:

> 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

The balancing fix has 3 main problems imo:
- It causes long delays to receive the notification (i.e. with a timeout of 30s 
the caller may be notified after 30s in the worst case)
- It doesn't apply to other affected APIs, e.g. Queue.get() which uses a 
condition internally.
- It only fixes the problem in python programs which explicitly use it (and 
being redhat specific i guess it is not very used)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[STINNER Victor]

STINNER Victor added the comment:

"The balancing fix has 3 main problems imo:"

Oh by the way, I don't want to apply it to Python 2.7 upstream for a
similar reason: it's a new feature which would be added to a Python
2.7 minor version. We try to avoid that, even if Python 2.7 is
special.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Antoine Pitrou]

Antoine Pitrou added the comment:

Ok, let's close it then.

--
resolution:  -> wont fix
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Nick Coghlan]

Nick Coghlan added the comment:

The consistency issue I was referring to related to the "timeout" argument - we 
currently have a patch applied to Fedora (and derivatives) that instead adds a 
"balanced=True" flag argument so you can pass "polling=False" to turn off the 
busy loop (at the risk of a long wake-up delay).

This means the current situation we have in Fedora (et al) is:

* Python 2 only code can pass "balanced=False" (but probably wouldn't want to 
due to the potential increase in wake-up latency)
* Python 3 only code can specify a timeout directly
* Single source Python 2/3 code can't do either (since the API signatures are 
different)

So downstream I'll be advocating in favour of Flavio's pthread patch regardless 
of what we do upstream - that way hybrid Python 2/3 code that's specific to the 
Fedora derived ecosystem (and we have a lot of that in the operating system 
layer) can eventually be updated to use threading timeouts while remaining 
consistent between Fedora (which is switching the system Python to Python 3) 
and RHEL/CentOS 7 (which will continue to use Python 2.7).

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

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Antoine Pitrou]

Antoine Pitrou added the comment:

Yes.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[STINNER Victor]

STINNER Victor added the comment:

Nick, Antoine: do you agree to close this issue as WONTFIX for the reasons 
explained in msg250712?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Flavio Grossi]

Flavio Grossi added the comment:

First of all, thank you for your support.

I fully understand what you are saying, however, being stuck to python 2.7 
because of libraries still not ported to 3, this is a serious problem to us, 
and, while i agree this would introduce a new "feature", it also fixes a bug 
which in some cases renders some functionalities (Queue.get() for example) not 
very usable as it is.

Is there any chance this will be fixed? Or even having the remaining thread 
implementations fixed will lead to reject this?

Thank you

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[matteo]

Changes by matteo :


--
nosy: +matteo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[STINNER Victor]

STINNER Victor added the comment:

2015-09-15 9:17 GMT+02:00 Flavio Grossi :
> however, being stuck to python 2.7 because of libraries still not ported to 
> 3, this is a serious problem to us,

Are there private libraries or public libraries? If there are public,
what are these libraries. I ported a lot of libraries last year, more
and more libraries are compatible with Python 3.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Flavio Grossi]

Flavio Grossi added the comment:

>> however, being stuck to python 2.7 because of libraries 
> Are there private libraries or public libraries?

It is a mix of public and internal libraries with the main public blockers 
being twisted and apache thrift (and other libs which have py3 alternatives but 
don't have retrocompatible apis).

(Yes, twisted is almost supported, but still has some parts not ready for 
python 3)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Nick Coghlan]

Nick Coghlan added the comment:

For the sake of folks writing single-source code, and needing to support Python 
2.7 for at least as long as we're supporting it upstream, I believe it would be 
beneficial to have consistency here.

For those that didn't follow the Fedora/RHEL issue chain, the original Fedora 
19 bug report was https://bugzilla.redhat.com/show_bug.cgi?id=917709 and the 
corresponding "won't fix" upstream issue was issue 17748.

Unfortunately, in addition to only helping in a subset of cases, the workaround 
we applied also extends the affected APIs in an undocumented way that's 
incompatible with the Python 3 changes to the same API, so I've suggested that 
regardless of the verdict upstream, we should bring the API extension into line 
with Python 3 downstream.

--
nosy: +bkabrda, ncoghlan, rkuska

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Antoine Pitrou]

Antoine Pitrou added the comment:

There's no consistency issue here, Python 3 just shows better performance and 
resource usage. Python 2 has always had the busy polling implementation.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[STINNER Victor]

STINNER Victor added the comment:

As you noticed, this issue was correctly fixed in Python 3 by using the C timed 
acquire methods of locks, instead of polling. The problem is that Python 2 
supports a wide range of threading models:

Python/thread_atheos.h
Python/thread_beos.h
Python/thread_cthread.h
Python/thread_lwp.h
Python/thread_nt.h
Python/thread_os2.h
Python/thread_pth.h
Python/thread_pthread.h
Python/thread_sgi.h
Python/thread_solaris.h
Python/thread_wince.h

In Python 3, it was possible to use more features of OS threads because we 
dropped all implementations to only keep the 2 major implementations:

Python/thread_nt.h
Python/thread_pthread.h

Your change adds a new feature to threading.Lock in Python 2:

-.. method:: Lock.acquire([blocking])
+.. method:: Lock.acquire(blocking=True, timeout=-1)

But features cannot be added to Python 2 anymore!

The backport changes critical C code. There is a (high) risk of introducing a 
regression.

I suggest to close this issue as WONTFIX.

@Benjamin (Python 2.7 release manager): What do you think?

In 2015, it's time to upgrade to Python 3! 
https://docs.python.org/3/howto/pyporting.html

--
nosy: +benjamin.peterson, haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25084] remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)

[Flavio Grossi]

New submission from Flavio Grossi:

threading.Condition.wait(timeout=x) is implemented in python 2  as a semi-busy 
loop which causes cpu wakeups and unexpected cpu use.

I think this is somewhat problematic since it causes problems in all modules 
which use that method, such as Queue.get() when used with a timeout.

This issue has been reported and "fixed" in red hat based distributions in a 
way which i find problematic, as detailed here:
https://bugzilla.redhat.com/show_bug.cgi?id=1230802

The attached patch backports the following change from py3 to fix the problem:
https://hg.python.org/cpython/rev/01d1fd775d16/

--
components: Library (Lib)
files: timedlock.patch
keywords: patch
messages: 250562
nosy: flavio, pitrou
priority: normal
severity: normal
status: open
title: remove semi-busy loop in py2.7 threading.Condition.wait(timeout=x)
type: enhancement
versions: Python 2.7
Added file: http://bugs.python.org/file40450/timedlock.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com