classification
Title: concurrent.futures uses polling
Type: resource usage Stage: committed/rejected
Components: Library (Lib) Versions: Python 3.3, Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: bquinlan, gregory.p.smith, jyasskin, pitrou, python-dev, s7v7nislands, stutzbach
Priority: normal Keywords: patch

Created on 2011-03-22 17:36 by pitrou, last changed 2011-03-26 18:37 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
cfpolling2.patch pitrou, 2011-03-23 00:56
cfpolling3.patch pitrou, 2011-03-23 20:53
cfpolling4.patch pitrou, 2011-03-24 15:14
cfpolling5.patch pitrou, 2011-03-24 18:34
Messages (16)
msg131756 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-22 17:36
concurrent.futures uses polling in its worker threads and processes
(with a timeout of 0.1).
It means that:
1) this prevents CPUs to enter low power states durably
2) it incurs latency when calling shutdown() on an executor (this seems to be the main source of slowness in test_concurrent_futures under Linux)
msg131831 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-23 00:56
Ok, here is a patch. Summary:
- leave a minimal amount of polling (every 600 seconds) to avoid blocking forever if there's a bug (shouldn't happen of course, but who knows? especially with multiprocessing)
- when wanting to wakeup a worker, put None in its receiving queue
- remove periodic cleanup of thread references by using a weak dict instead
- in tests, compute runtime and make the test fail if the runtime exceeds 60 seconds (to report aforementioned synchronization bugs)

Tested under Linux (extensively) and Windows 7 (briefly).
msg131833 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-23 01:05
(patch is for 3.2, by the way. Perhaps this should only be fixed in default?)
msg131840 - (view) Author: Brian Quinlan (bquinlan) (Python committer) Date: 2011-03-23 02:26
I would suggest that you base your patch on 3.3/default.
msg131841 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-23 02:28
> I would suggest that you base your patch on 3.3/default.

Well, since the module is new, I think it would be nice to fix such
quirks in the bugfix branch as well. So, following the recommended
workflow, I've started with a 3.2 patch.
msg131861 - (view) Author: Brian Quinlan (bquinlan) (Python committer) Date: 2011-03-23 09:38
Your approach seems workable but your patch allows the interpreter to exit while work items are still being processed. See the comment at the top of concurrent/futures/thread.py.
msg131867 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-23 13:40
> Your approach seems workable but your patch allows the interpreter to
> exit while work items are still being processed. See the comment at
> the top of concurrent/futures/thread.py.

Why are you saying that? In my patch, _python_exit() still takes care of
joining worker threads.
msg131913 - (view) Author: Brian Quinlan (bquinlan) (Python committer) Date: 2011-03-23 20:00
Sorry, I didn't read an error message very carefully. When I apply your patch I see:

>>> from concurrent.futures import *
>>> from time import *
>>> t = ThreadPoolExecutor(5)
>>> t.submit(sleep, 100)
<Future at 0x8acc94c state=running>
>>> <ctrl-D>
Error in atexit._run_exitfuncs:
NameError: global name 'thread' is not defined

Does that not happen in your environment?
msg131914 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-23 20:04
> Sorry, I didn't read an error message very carefully. When I apply your patch I see:
> 
> >>> from concurrent.futures import *
> >>> from time import *
> >>> t = ThreadPoolExecutor(5)
> >>> t.submit(sleep, 100)
> <Future at 0x8acc94c state=running>
> >>> <ctrl-D>
> Error in atexit._run_exitfuncs:
> NameError: global name 'thread' is not defined
> 
> Does that not happen in your environment?

Ah, thank you, it does happen here too. I should fix the error (a typo)
and add tests for this too.
msg131918 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-23 20:39
Ok, here is a new patch with an additional test for the atexit hook. If you don't object, I would like to start committing the test changes, and then the code changes themselves.
msg131920 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-23 20:53
Oops, test didn't work under Windows. Here is a new patch.
msg131979 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-24 14:49
New changeset 76a898433a02 by Antoine Pitrou in branch '3.2':
Add tests for the atexit hook in concurrent.futures (part of #11635)
http://hg.python.org/cpython/rev/76a898433a02

New changeset d6bbde982c1c by Antoine Pitrou in branch 'default':
Add tests for the atexit hook in concurrent.futures (part of #11635)
http://hg.python.org/cpython/rev/d6bbde982c1c
msg131986 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-24 15:14
Tests now committed, here is a patch without them.
msg132012 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-24 18:34
After studying the multiprocessing code, it turns out that Queue.get() with a timeout does its own rather high-frequency polling under Windows (see Modules/_multiprocessing/pipe_connection.c). Therefore, here is an updated patch which doesn't have a security timeout at all.
msg132261 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-26 18:34
New changeset 4390d6939a56 by Antoine Pitrou in branch '3.2':
Issue #11635: Don't use polling in worker threads and processes launched by
http://hg.python.org/cpython/rev/4390d6939a56

New changeset a76257a99636 by Antoine Pitrou in branch 'default':
Issue #11635: Don't use polling in worker threads and processes launched by
http://hg.python.org/cpython/rev/a76257a99636
msg132262 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-26 18:37
I've now pushed the patch. I hope this won't break anything, closing.
History
Date User Action Args
2011-03-26 18:37:06pitrousetstatus: open -> closed
resolution: fixed
messages: + msg132262

stage: patch review -> committed/rejected
2011-03-26 18:34:00python-devsetmessages: + msg132261
2011-03-26 09:07:32s7v7nislandssetnosy: + s7v7nislands
2011-03-24 18:34:28pitrousetfiles: + cfpolling5.patch

messages: + msg132012
2011-03-24 15:14:50pitrousetfiles: + cfpolling4.patch

messages: + msg131986
2011-03-24 14:49:10python-devsetnosy: + python-dev
messages: + msg131979
2011-03-23 20:53:42pitrousetfiles: - cfpolling3.patch
2011-03-23 20:53:33pitrousetfiles: + cfpolling3.patch

messages: + msg131920
2011-03-23 20:39:42pitrousetfiles: + cfpolling3.patch

stage: needs patch -> patch review
messages: + msg131918
versions: + Python 3.2
2011-03-23 20:04:22pitrousetmessages: + msg131914
2011-03-23 20:00:46bquinlansetmessages: + msg131913
2011-03-23 13:40:19pitrousetnosy: gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach
messages: + msg131867
2011-03-23 09:38:27bquinlansetnosy: gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach
messages: + msg131861
2011-03-23 02:28:32pitrousetnosy: gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach
messages: + msg131841
2011-03-23 02:26:46bquinlansetnosy: gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach
messages: + msg131840
2011-03-23 01:05:53pitrousetnosy: gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach
messages: + msg131833
2011-03-23 00:56:24pitrousetfiles: + cfpolling2.patch

messages: + msg131831
keywords: + patch
nosy: gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach
2011-03-22 20:43:23stutzbachsetnosy: + stutzbach
2011-03-22 17:36:39pitroucreate