msg131756 - (view) |
Author: Antoine Pitrou (pitrou) * |
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) * |
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) * |
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) * |
Date: 2011-03-23 02:26 |
I would suggest that you base your patch on 3.3/default.
|
msg131841 - (view) |
Author: Antoine Pitrou (pitrou) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2011-03-24 15:14 |
Tests now committed, here is a patch without them.
|
msg132012 - (view) |
Author: Antoine Pitrou (pitrou) * |
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) * |
Date: 2011-03-26 18:37 |
I've now pushed the patch. I hope this won't break anything, closing.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:15 | admin | set | github: 55844 |
2011-03-26 18:37:06 | pitrou | set | status: open -> closed resolution: fixed messages:
+ msg132262
stage: patch review -> resolved |
2011-03-26 18:34:00 | python-dev | set | messages:
+ msg132261 |
2011-03-26 09:07:32 | s7v7nislands | set | nosy:
+ s7v7nislands
|
2011-03-24 18:34:28 | pitrou | set | files:
+ cfpolling5.patch
messages:
+ msg132012 |
2011-03-24 15:14:50 | pitrou | set | files:
+ cfpolling4.patch
messages:
+ msg131986 |
2011-03-24 14:49:10 | python-dev | set | nosy:
+ python-dev messages:
+ msg131979
|
2011-03-23 20:53:42 | pitrou | set | files:
- cfpolling3.patch |
2011-03-23 20:53:33 | pitrou | set | files:
+ cfpolling3.patch
messages:
+ msg131920 |
2011-03-23 20:39:42 | pitrou | set | files:
+ cfpolling3.patch
stage: needs patch -> patch review messages:
+ msg131918 versions:
+ Python 3.2 |
2011-03-23 20:04:22 | pitrou | set | messages:
+ msg131914 |
2011-03-23 20:00:46 | bquinlan | set | messages:
+ msg131913 |
2011-03-23 13:40:19 | pitrou | set | nosy:
gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach messages:
+ msg131867 |
2011-03-23 09:38:27 | bquinlan | set | nosy:
gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach messages:
+ msg131861 |
2011-03-23 02:28:32 | pitrou | set | nosy:
gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach messages:
+ msg131841 |
2011-03-23 02:26:46 | bquinlan | set | nosy:
gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach messages:
+ msg131840 |
2011-03-23 01:05:53 | pitrou | set | nosy:
gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach messages:
+ msg131833 |
2011-03-23 00:56:24 | pitrou | set | files:
+ cfpolling2.patch
messages:
+ msg131831 keywords:
+ patch nosy:
gregory.p.smith, bquinlan, pitrou, jyasskin, stutzbach |
2011-03-22 20:43:23 | stutzbach | set | nosy:
+ stutzbach
|
2011-03-22 17:36:39 | pitrou | create | |