concurrent.futures uses polling

[pitrou]

BPO	11635
Nosy	@gpshead, @brianquinlan, @pitrou
Files	cfpolling2.patch cfpolling3.patch cfpolling4.patch cfpolling5.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2011-03-26.18:37:06.888>
created_at = <Date 2011-03-22.17:36:39.335>
labels = ['library', 'performance']
title = 'concurrent.futures uses polling'
updated_at = <Date 2011-03-26.18:37:06.887>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2011-03-26.18:37:06.887>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2011-03-26.18:37:06.888>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2011-03-22.17:36:39.335>
creator = 'pitrou'
dependencies = []
files = ['21351', '21361', '21373', '21378']
hgrepos = []
issue_num = 11635
keywords = ['patch']
message_count = 16.0
messages = ['131756', '131831', '131833', '131840', '131841', '131861', '131867', '131913', '131914', '131918', '131920', '131979', '131986', '132012', '132261', '132262']
nosy_count = 7.0
nosy_names = ['gregory.p.smith', 'bquinlan', 'pitrou', 'jyasskin', 'stutzbach', 's7v7nislands', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue11635'
versions = ['Python 3.2', 'Python 3.3']

[pitrou]

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)

[pitrou]

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

[pitrou]

(patch is for 3.2, by the way. Perhaps this should only be fixed in default?)

[brianquinlan]

I would suggest that you base your patch on 3.3/default.

[pitrou]

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.

[brianquinlan]

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.

[pitrou]

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.

[brianquinlan]

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?

[pitrou]

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.

[pitrou]

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.

[pitrou]

Oops, test didn't work under Windows. Here is a new patch.

[python-dev]

New changeset 76a898433a02 by Antoine Pitrou in branch '3.2':
Add tests for the atexit hook in concurrent.futures (part of bpo-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 bpo-11635)
http://hg.python.org/cpython/rev/d6bbde982c1c

[pitrou]

Tests now committed, here is a patch without them.