Skip to content

/ pythonfutures

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backport fixes to as_completed and map iterators (bpo-27144) #66

Merged
merged 1 commit into from Nov 29, 2017
Merged

Backport fixes to as_completed and map iterators (bpo-27144) #66

merged 1 commit into from Nov 29, 2017

Conversation

@dalcinl
Copy link
Contributor

@dalcinl dalcinl commented Oct 3, 2017

Python issues:
@dalcinl
Backport fixes to as_completed and map iterators (bpo-27144)
Loading status checks…
61f5abb 
Python issues:

+ python/cpython#1560
+ python/cpython#3270
+ python/cpython#3830
@dalcinl
Copy link
Contributor Author

@dalcinl dalcinl commented Oct 3, 2017

@agronholm As you mentioned you are busy, here you have a reproducer with output from runs before and after the changes in this PR. Note how the count (and obviously memory footprint) of bytearray instances keeps going up while you iterate over as_completed() and Executor.map() iterators.

Reproducer script:

# file: reproducer.py
import gc
import time
from pympler import summary
from pympler import muppy

from concurrent.futures import ThreadPoolExecutor, as_completed


def random_data(*args):
    time.sleep(1)
    return bytearray(1024*1024*2)


def gen_executor0():
    with ThreadPoolExecutor(1) as executor:
        futures = [executor.submit(random_data) for x in range(5)]
        for future in as_completed(futures):
            futures.remove(future)
            future = None
            yield

def gen_executor1():
    with ThreadPoolExecutor(1) as executor:
        for y in executor.map(random_data, range(5)):
            y = None
            yield 

def main(gen_executor):
    for cycle in gen_executor():
        gc.collect()
        summary.print_(summary.summarize(muppy.get_objects()), limit=4)

if __name__ == '__main__':
    print("as_completed()")
    main(gen_executor0)
    print("executor.map()")
    main(gen_executor1)

Run before this PR:

$ python reproducer.py
as_completed()
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           1 |      2.00 MB
       dict |         767 |      1.14 MB
        str |        8468 |    803.97 KB
       code |        2595 |    324.38 KB
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           2 |      4.00 MB
       dict |         768 |      1.14 MB
        str |        8468 |    803.97 KB
       code |        2595 |    324.38 KB
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           3 |      6.00 MB
       dict |         767 |      1.14 MB
        str |        8468 |    803.97 KB
       code |        2595 |    324.38 KB
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           4 |      8.00 MB
       dict |         766 |      1.14 MB
        str |        8468 |    803.97 KB
       code |        2595 |    324.38 KB
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           5 |     10.00 MB
       dict |         765 |      1.14 MB
        str |        8466 |    803.89 KB
       code |        2595 |    324.38 KB
executor.map()
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           1 |      2.00 MB
       dict |         766 |      1.14 MB
        str |        8465 |    803.84 KB
       code |        2596 |    324.50 KB
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           2 |      4.00 MB
       dict |         765 |      1.14 MB
        str |        8465 |    803.84 KB
       code |        2596 |    324.50 KB
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           3 |      6.00 MB
       dict |         764 |      1.14 MB
        str |        8465 |    803.84 KB
       code |        2596 |    324.50 KB
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           4 |      8.00 MB
       dict |         763 |      1.14 MB
        str |        8465 |    803.84 KB
       code |        2596 |    324.50 KB
      types |   # objects |   total size
=========== | =========== | ============
  bytearray |           5 |     10.00 MB
       dict |         762 |      1.14 MB
        str |        8463 |    803.76 KB
       code |        2596 |    324.50 KB

Run after this PR:

$ python reproducer.py
as_completed()
  types |   # objects |   total size
======= | =========== | ============
   dict |         765 |      1.14 MB
    str |        8470 |    804.44 KB
   code |        2596 |    324.50 KB
   type |         167 |    148.64 KB
  types |   # objects |   total size
======= | =========== | ============
   dict |         764 |      1.14 MB
    str |        8470 |    804.44 KB
   code |        2596 |    324.50 KB
   type |         167 |    148.64 KB
  types |   # objects |   total size
======= | =========== | ============
   dict |         761 |      1.14 MB
    str |        8470 |    804.44 KB
   code |        2596 |    324.50 KB
   type |         167 |    148.64 KB
  types |   # objects |   total size
======= | =========== | ============
   dict |         758 |      1.13 MB
    str |        8470 |    804.44 KB
   code |        2596 |    324.50 KB
   type |         167 |    148.64 KB
  types |   # objects |   total size
======= | =========== | ============
   dict |         755 |      1.13 MB
    str |        8461 |    804.04 KB
   code |        2596 |    324.50 KB
   type |         167 |    148.64 KB
executor.map()
  types |   # objects |   total size
======= | =========== | ============
   dict |         764 |      1.14 MB
    str |        8467 |    804.30 KB
   code |        2597 |    324.62 KB
   type |         167 |    148.64 KB
  types |   # objects |   total size
======= | =========== | ============
   dict |         761 |      1.14 MB
    str |        8467 |    804.30 KB
   code |        2597 |    324.62 KB
   type |         167 |    148.64 KB
  types |   # objects |   total size
======= | =========== | ============
   dict |         758 |      1.13 MB
    str |        8467 |    804.30 KB
   code |        2597 |    324.62 KB
   type |         167 |    148.64 KB
  types |   # objects |   total size
======= | =========== | ============
   dict |         755 |      1.13 MB
    str |        8467 |    804.30 KB
   code |        2597 |    324.62 KB
   type |         167 |    148.64 KB
  types |   # objects |   total size
======= | =========== | ============
   dict |         752 |      1.13 MB
    str |        8458 |    803.91 KB
   code |        2597 |    324.62 KB
   type |         167 |    148.64 KB
@agronholm agronholm mentioned this pull request Nov 29, 2017
Closed
@agronholm
agronholm reviewed Nov 29, 2017
View changes
Copy link
Owner

@agronholm agronholm left a comment

Two oddities, otherwise looks good.
concurrent/futures/_base.py
yield future
for f in _yield_finished_futures(finished, waiter,
ref_collect=(fs,)):
f = [f]

This comment has been minimized.

Sign in to view
@agronholm

agronholm Nov 29, 2017
Owner

Why this? Why not just yield f?

This comment has been minimized.

Sign in to view
@agronholm

agronholm Nov 29, 2017
Owner

Ahhh...because otherwise the future would still be referenced by the generator. Gotcha.

This comment has been minimized.

Sign in to view
@dalcinl

dalcinl Nov 29, 2017
Author Contributor

Indeed. Maybe Python 4 will consider fixing iteration target vars leaking to the scope :-).
concurrent/futures/_base.py
finished.reverse()
for f in _yield_finished_futures(finished, waiter,
ref_collect=(fs, pending)):
f = [f]

This comment has been minimized.

Sign in to view
@agronholm

agronholm Nov 29, 2017
Owner

Ditto
@agronholm agronholm merged commit 28fa404 into agronholm:master Nov 29, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@agronholm
Copy link
Owner

@agronholm agronholm commented Nov 29, 2017

Thank you very much!
@dalcinl dalcinl deleted the dalcinl:bpo-27144 branch Nov 29, 2017
agronholm added a commit that referenced this pull request Nov 30, 2017
@agronholm
Added notes from PR #66
552ca07
@cowbert cowbert mentioned this pull request May 12, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agronholm agronholm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@dalcinl @agronholm
You can’t perform that action at this time.