Skip to content
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

BUG: Memory leaks in numpy.nested_iters #22296

Merged
merged 1 commit into from Sep 18, 2022

Conversation

clearspear
Copy link
Contributor

@clearspear clearspear commented Sep 16, 2022

Bug fix for this issue: #19400 as part of the Grace Hopper OSD NumPy sprint

Copy link
Contributor

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

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

Taking the core reproducer from the matching issue:

import numpy as np

for i in range(0, 1000):
    try:
        np.nested_iters(['e', 'f'], [[1], [2]])
    except:
        pass

On main with memory_profiler:

image

On this feature branch it seems to be identical:

image

For the original tracemalloc reproducer, this branch does look much better:

/usr/lib/python3.10/tracemalloc.py:547
1256

/usr/lib/python3.10/tracemalloc.py:547
1256

/usr/lib/python3.10/tracemalloc.py:547
1256

/usr/lib/python3.10/tracemalloc.py:547
1256

/usr/lib/python3.10/tracemalloc.py:547
1256

/usr/lib/python3.10/tracemalloc.py:547
1256

/usr/lib/python3.10/tracemalloc.py:547
1256

/usr/lib/python3.10/tracemalloc.py:547
1256

/usr/lib/python3.10/tracemalloc.py:547
1256

whereas on latest main we still leak:

/home/treddy/rough_work/numpy/pr_22296/test.py:10
16160

/home/treddy/rough_work/numpy/pr_22296/test.py:10
32160

/home/treddy/rough_work/numpy/pr_22296/test.py:10
48160

/home/treddy/rough_work/numpy/pr_22296/test.py:10
64160

/home/treddy/rough_work/numpy/pr_22296/test.py:10
80160

/home/treddy/rough_work/numpy/pr_22296/test.py:10
96208

/home/treddy/rough_work/numpy/pr_22296/test.py:10
112208

/home/treddy/rough_work/numpy/pr_22296/test.py:10
128160

/home/treddy/rough_work/numpy/pr_22296/test.py:10
144160

So, this looks solid IMO. I'm not even sure that we'd require a regression test in this case--I suspect not. I do wonder if we could easily add the reproducer from the issue--it runs very quickly, but I'm not so sure we want tracemalloc stuff in our test suite. Cool "idea" from the fuzzer at least.

@tylerjereddy tylerjereddy merged commit 05f6459 into numpy:main Sep 18, 2022
@tylerjereddy
Copy link
Contributor

thanks for your first contribution to NumPy

@InessaPawson
Copy link
Member

Hi-five on merging your first pull request to NumPy, @clearspear! We hope you stick around! Your choices aren’t limited to programming – you can review pull requests, help us stay on top of new and old issues, develop educational material, work on our website, add or improve graphic design, create marketing materials, translate website content, write grant proposals, and help with other fundraising initiatives. For more info, check out: https://numpy.org/contribute
Also, consider joining our mailing list. This is a great way to connect with other cool people in our community and be part of important conversations that affect the development of NumPy: https://mail.python.org/mailman/listinfo/numpy-discussion

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Oct 6, 2022
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants