bpo-29842: Make Executor.map less eager so it handles large/unbounded…#707
bpo-29842: Make Executor.map less eager so it handles large/unbounded…#707MojoVampire wants to merge 6 commits into
Conversation
… input iterables appropriately
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:
Thanks again to your contribution and we look forward to looking at it! |
Sorry, something went wrong.
|
@MojoVampire, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @brianquinlan and @ezio-melotti to be potential reviewers. |
Sorry, something went wrong.
|
I've already submitted a contributor agreement pre-GitHub migration. I just updated my b.p.o. user profile (josh.r) to link to my GitHub account name. Is that sufficient, or do I need to submit a new contributor agreement based on my GitHub e-mail address? |
Sorry, something went wrong.
|
Hmm... Is the failure of continuous-integration/travis-ci/pr something real? Clicking Details just tells me it can't find a python/cpython repository at all... |
Sorry, something went wrong.
|
You can also take a look at my implementation that I uploaded to https://github.com/pkch/executors. It does something more like what I described in the issue tracker, the main benefit being that it's not blocking. |
Sorry, something went wrong.
methane
left a comment
There was a problem hiding this comment.
LGTM at code level.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
|
@MojoVampire could you share your plans about this PR? Do you plan to drive it forward? |
Sorry, something went wrong.
|
I have made the requested changes; please review again. I did not add a Misc/NEWS entry since the file no longer exists (it's autogenerated from commit messages now, correct?). |
Sorry, something went wrong.
|
Thanks for making the requested changes! @methane: please review the changes made to this pull request. |
Sorry, something went wrong.
NEWS entries can be generated using blurb or blurb-it Please see : https://devguide.python.org/committing/?highlight=news#what-s-new-and-news-entries |
Sorry, something went wrong.
|
I have made the requested changes; please review again. Actually made the Misc/NEWS entry properly. Sorry for confusion; I haven't made a PR since the news Misc/NEWS regime began and didn't know about the blurb tool. Thanks for the assist @tirkarthi |
Sorry, something went wrong.
|
Thanks for making the requested changes! @methane: please review the changes made to this pull request. |
Sorry, something went wrong.
pitrou
left a comment
There was a problem hiding this comment.
Some comments below. A significant issue is that this changes the behaviour of shutdown(wait=True) to not wait for completion of all pending futures. I don't think that's an acceptable change.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
…lds no reference to result at the moment it yields Reduce line lengths to PEP8 limits
|
@MojoVampire I think you need to comment your PR with |
Sorry, something went wrong.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
|
@erlend-aasland: I'd like to apply this, but I never got any idea of what would constitute an acceptable final result. Executor.map is, frankly, useless for many of its intended purposes right now; in an effort to improve performance on huge inputs, you end up prefetching the entire input and pre-scheduling all the tasks before you can process any of them. |
Sorry, something went wrong.
|
cc. @kumaraditya303, is this something you'd be interested in reviewing? |
Sorry, something went wrong.
I'll take a look soon, but it seems there are two PRs for the same thing and this one has conflicts so maybe we should continue on #18566 |
Sorry, something went wrong.
… input iterables appropriately
https://bugs.python.org/issue29842
https://bugs.python.org/issue29842