Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(82150)

#11271: concurrent.futures.ProcessPoolExecutor.map() doesn't batch function arguments by chunks

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by tobias.brink
Modified:
5 years ago
Reviewers:
pitrou, oreilldf, cf.natali
CC:
bquinlan, AntoinePitrou, Charles-François Natali, tobias.brink_gmail.com, devnull_psf.upfronthosting.co.za, sbt, Josh.R, dan.oreilly
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Total comments: 4

Patch Set 4 #

Total comments: 4

Patch Set 5 #

Total comments: 7

Patch Set 6 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/concurrent.futures.rst View 1 2 3 4 5 2 chunks +8 lines, -2 lines 0 comments Download
Lib/concurrent/futures/_base.py View 1 2 3 4 5 2 chunks +5 lines, -1 line 0 comments Download
Lib/concurrent/futures/process.py View 1 2 3 4 5 3 chunks +54 lines, -0 lines 0 comments Download
Lib/test/test_concurrent_futures.py View 1 2 3 4 5 1 chunk +17 lines, -0 lines 0 comments Download

Messages

Total messages: 7
AntoinePitrou
http://bugs.python.org/review/11271/diff/12473/Lib/concurrent/futures/process.py File Lib/concurrent/futures/process.py (right): http://bugs.python.org/review/11271/diff/12473/Lib/concurrent/futures/process.py#newcode115 Lib/concurrent/futures/process.py:115: it = [zip(*iterables)] * chunksize This code looks really ...
5 years ago #1
dan.oreilly
http://bugs.python.org/review/11271/diff/12473/Lib/concurrent/futures/process.py File Lib/concurrent/futures/process.py (right): http://bugs.python.org/review/11271/diff/12473/Lib/concurrent/futures/process.py#newcode115 Lib/concurrent/futures/process.py:115: it = [zip(*iterables)] * chunksize Even worse, there's a ...
5 years ago #2
AntoinePitrou
Thanks for the updated patch. A couple comments below. Also, this still misses: - a ...
5 years ago #3
dan.oreilly
Just to clarify, your preference for ThreadPoolExecutor is to simply make the API match ProcessPoolExecutor ...
5 years ago #4
AntoinePitrou
Thank you very much! I only have a single, quite trivial comment, so I can ...
5 years ago #5
Charles-François Natali
http://bugs.python.org/review/11271/diff/12656/Lib/concurrent/futures/process.py File Lib/concurrent/futures/process.py (right): http://bugs.python.org/review/11271/diff/12656/Lib/concurrent/futures/process.py#newcode436 Lib/concurrent/futures/process.py:436: def map(self, fn, *iterables, timeout=None, chunksize=1): Why not use ...
5 years ago #6
dan.oreilly
5 years ago #7
http://bugs.python.org/review/11271/diff/12656/Lib/concurrent/futures/process.py
File Lib/concurrent/futures/process.py (right):

http://bugs.python.org/review/11271/diff/12656/Lib/concurrent/futures/process...
Lib/concurrent/futures/process.py:436: def map(self, fn, *iterables,
timeout=None, chunksize=1):
This is by request of Antoine (http://bugs.python.org/issue11271#msg137692). He
said:

"Also, the chunksize can't be anything else than 1 by default, since your
approach is increasing latency of returning results."

On 2014/08/11 15:55:10, Charles-François Natali wrote:
> Why not use a heuristic similar to multiprocessing.Pool for the default
> chunksize?

http://bugs.python.org/review/11271/diff/12656/Lib/concurrent/futures/process...
Lib/concurrent/futures/process.py:460: def result_iterator():
Yep, it would. I'll make that change.

On 2014/08/11 15:55:10, Charles-François Natali wrote:
> Wouldn't itertools.chain(result) do the same thing?

http://bugs.python.org/review/11271/diff/12656/Lib/test/test_concurrent_futur...
File Lib/test/test_concurrent_futures.py (right):

http://bugs.python.org/review/11271/diff/12656/Lib/test/test_concurrent_futur...
Lib/test/test_concurrent_futures.py:464: list(map(pow, range(40), range(40))))
Sounds reasonable.

On 2014/08/11 15:55:10, Charles-François Natali wrote:
> It would be nice to test several values (the reason I'm asking is because I've
> been playing with billiard recently, and it blows up depending on the chunk
size
> you provide).
> 
> Some interesting values:
> - < 1 (since it's supposed to raise a ValueError)
> - <= len(iterable)
> - > len(iterable)
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+