GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-29842: Make Executor.map less eager so it handles large/unbounded… by graingert · Pull Request #18566 · python/cpython

Conversation

@graingert

@graingert graingert commented Feb 20, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

… input iterables appropriately

bugs.python.org/issue29842

bugs.python.org/issue29842

recreate of #707 with conflicts fixed and versionchanged updated to py3.9

/cc @MojoVampire

https://bugs.python.org/issue29842

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

d-ryzhykau, mhils, skshetry, its-wizza, leezu, illagrenan, graingert, Tronic, b0ri5, davystrong, and 5 more reacted with thumbs up emoji
All reactions
  • 👍 15 reactions
MojoVampire and others added 8 commits March 17, 2017 22:05
@MojoVampire
bpo-29842: Make Executor.map less eager so it handles large/unbounded…
fbdb56c
… input iterables appropriately
@MojoVampire
Merge branch 'master' into fix-issue-29842
6281379
@MojoVampire
Add prefetch info to docstrings
8394e34
@MojoVampire
Add Misc/NEWS entry
634a3de
@MojoVampire
Remove trailing whitespace in docs
0152831
@MojoVampire
Fix behavior to follow test_free_reference requirements (generator ho…
3c38fab
…lds no reference to result at the moment it yields

Reduce line lengths to PEP8 limits
@graingert
Merge branch 'master' of git://github.com/python/cpython into fix-iss…
d32de88
…ue-29842
@graingert
updated concurrent.futures:ThreadPoolExecutor.map prefetch versionadded
f3a3791
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 20, 2020
@bedevere-bot bedevere-bot added the awaiting review label Feb 20, 2020
rdarder
rdarder reviewed Mar 22, 2021
View reviewed changes

@rdarder rdarder left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there! sorry for intruding. I was looking for this feature and decided to try this patch out. Works great! Just found a small issue so I sent you a comment.
Thank you!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

its-wizza reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Lib/concurrent/futures/_base.py
except StopIteration:
argsiter = None
else:
fs.append(self.submit(fn, *args))

@rdarder rdarder Mar 22, 2021

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the executor has been shut down, this will raise:
cannot schedule new futures after shutdown
But, also the base executor holds no state, so at this level it'll be pretty hard to tell if the executor has been shut down or not.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

its-wizza and graingert reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

@graingert graingert Jul 23, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rdarder
this could catch that RuntimeError and just return, or the executor could keep a weakset of result_iterators and close them on shutdown

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@graingert
Merge branch 'main' into fix-issue-29842
eb8d26a
graingert
graingert commented Jul 21, 2021
View reviewed changes
Comment thread Doc/library/concurrent.futures.rst Outdated
.. versionchanged:: 3.5
Added the *chunksize* argument.

.. versionchanged:: 3.9

@graingert graingert Jul 21, 2021

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. versionchanged:: 3.9
.. versionchanged:: 3.11

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
graingert
graingert commented Jul 21, 2021
View reviewed changes
Comment thread Doc/library/concurrent.futures.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@graingert
Update Doc/library/concurrent.futures.rst
9234a48
@Tronic

Tronic commented Oct 29, 2021

Copy link
Copy Markdown

While waiting for this to be included in Python, one can easily copy&paste the map function out of this version and call it with the executor as the first argument. Hoping that you get this merged soon!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MojoVampire MojoVampire added performance Performance or resource usage stdlib Standard Library Python modules in the Lib/ directory labels May 4, 2022
@MojoVampire MojoVampire mannequin mentioned this pull request Apr 10, 2022
Make Executor.map work with infinite/large inputs correctly #74028
Closed
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@kumaraditya303 kumaraditya303 mentioned this pull request Jul 15, 2022
bpo-29842: Make Executor.map less eager so it handles large/unbounded… #707
Closed
@graingert
Merge branch 'main' into fix-issue-29842
74ec669
@kumaraditya303
kumaraditya303 self-requested a review July 18, 2022 15:30
graingert
graingert commented Jul 18, 2022
View reviewed changes
Comment thread Doc/library/concurrent.futures.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@graingert
Update Doc/library/concurrent.futures.rst
b618b7f
@kumaraditya303 kumaraditya303 added type-feature A feature request or enhancement 3.12 only security fixes labels Jul 18, 2022
@graingert
Merge branch 'main' into fix-issue-29842
13df4cd
@kumaraditya303

kumaraditya303 commented Jul 23, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

@graingert Tests are failing, can you fix them and rebase to main? I'll then review and run through the buildbots.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jul 23, 2022
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2019-05-06-12-49-21.bpo-29842.Tw-ETh.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

graingert and others added 4 commits July 23, 2022 10:13
@graingert @kumaraditya303
Update Misc/NEWS.d/next/Library/2019-05-06-12-49-21.bpo-29842.Tw-ETh.rst
bf36a8d
Co-authored-by: Kumar Aditya <[email protected]>
@graingert
Merge branch 'main' into fix-issue-29842
5cd250f
@graingert
use popleft instead of fs[0]
3cc0b4e
@graingert
Merge branch 'main' into fix-issue-29842
d33dca0
graingert
graingert commented Jul 23, 2022
View reviewed changes
Comment thread Lib/concurrent/futures/_base.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@graingert
Update Lib/concurrent/futures/_base.py
f058908
graingert
graingert commented Jul 23, 2022
View reviewed changes
Comment thread Lib/concurrent/futures/_base.py
yield fs.pop().result()
res = fs.popleft().result()
else:
yield fs.pop().result(end_time - time.monotonic())

@graingert graingert Jul 23, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't cancel the currently waited on fut correctly: #95166

import contextlib
import functools
import concurrent.futures
import threading
import sys


def fn(num, stop_event):
    if num == 1:
        stop_event.wait()
        return "done 1"

    if num == 2:
        return "done 2"


def main():
    stop_event = threading.Event()
    log = []

    with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool:

        def print_n_wait(ident):
            log.append(f"{ident=} started")
            try:
                stop_event.wait()
            finally:
                log.append(f"{ident=} stopped")

        fut = pool.submit(print_n_wait, ident="first")
        try:
            with contextlib.closing(pool.map(print_n_wait, ["second", "third"], timeout=1)) as gen:
                try:
                    next(gen)
                except concurrent.futures.TimeoutError:
                    print("timed out")
                else:
                    raise RuntimeError("timeout expected")
        finally:
            stop_event.set()

    assert log == ["ident='first' started", "ident='first' stopped"], f"{log=} is wrong"

if __name__ == "__main__":
    sys.exit(main())

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@kumaraditya303

kumaraditya303 commented Jul 23, 2022

Copy link
Copy Markdown
Contributor

@graingert You can run make patchcheck to fix the docs CI.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@SvenDS9 SvenDS9 mentioned this pull request Feb 24, 2023
Add ThreadMapperIterDatapipe meta-pytorch/data#1045
Closed
@Jason-Y-Z Jason-Y-Z mentioned this pull request Feb 3, 2024
gh-74028: Introduce a prefetch parameter to Executor.map to handle large iterators #114975
Closed
@Jason-Y-Z

Jason-Y-Z commented Feb 3, 2024

Copy link
Copy Markdown
Contributor

Hey all, I took the inspiration from this PR and continued the work in #114975

Tronic reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ebonnal ebonnal mentioned this pull request Oct 17, 2024
gh-74028: concurrent.futures.Executor.map: introduce buffersize param for lazier behavior #125663
Merged
@ebonnal

ebonnal commented Oct 17, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Hi, fyi here is a follow up PR: #125663 🙏🏻

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@graingert graingert closed this Dec 28, 2024
@graingert
graingert deleted the fix-issue-29842 branch December 28, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
+1 more reviewer
@rdarder rdarder rdarder left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

3.12 only security fixes awaiting review performance Performance or resource usage stdlib Standard Library Python modules in the Lib/ directory type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

@graingert @Tronic @kumaraditya303 @Jason-Y-Z @ebonnal @rdarder @the-knights-who-say-ni @ezio-melotti @MojoVampire @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.