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

Memory leak (with reproducer) when returning BTreeSet and HashSet from pyfunction #3285

Closed
cjrh opened this issue Jul 2, 2023 · 7 comments · Fixed by #3286
Closed

Memory leak (with reproducer) when returning BTreeSet and HashSet from pyfunction #3285

cjrh opened this issue Jul 2, 2023 · 7 comments · Fixed by #3286
Labels

Comments

@cjrh
Copy link

cjrh commented Jul 2, 2023

Bug Description

Memory is leaked when returning a BTreeSet or HashSet from a pyfunction in an extension module. (Vec and *Map are ok).

Steps to Reproduce

Overview

  1. Make a new PyO3 project (maturin init)
  2. Make a #[pyfunction] that returns PyResult<BTreeSet<u64>>
  3. Build and install
  4. Call this function repeatedly in a loop in Python code

Detail

The attached archive (pyo3mem.zip) contains a full project that can reproduce the issue.

pyo3mem.zip

To execute the reproducer:

  1. Extract the archive
  2. Create and activate a Python 3.10 venv
  3. Install the provided requirements_dev.txt: $ pip install -r requirements_dev.txt
  4. Run the tests: $ maturin develop && pytest -s

This is the output from stdout:

Returning a Vec
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         25018368         26230784          1212416
0001         26230784         26361856           131072
0002         26361856         26361856                0
0003         26361856         26361856                0
0004         26361856         26361856                0
0005         26361856         26361856                0
0006         26361856         26361856                0
0007         26361856         26361856                0
0008         26361856         26361856                0
0009         26361856         26361856                0
.

Returning a BTreeSet
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         26361856         30117888          3756032
0001         30117888         39555072          9437184
0002         39555072         42700800          3145728
0003         42700800         45977600          3276800
0004         45977600         49123328          3145728
0005         49123328         52400128          3276800
0006         52400128         55545856          3145728
0007         55545856         58822656          3276800
0008         58822656         61968384          3145728
0009         61968384         65245184          3276800
.

Returning a HashSet
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         65245184         68390912          3145728
0001         68390912         71667712          3276800
0002         71667712         74813440          3145728
0003         74813440         78090240          3276800
0004         78090240         81235968          3145728
0005         81235968         84512768          3276800
0006         84512768         87658496          3145728
0007         87658496         90935296          3276800
0008         90935296         94081024          3145728
0009         94081024         97357824          3276800
.

Returning a BTreeMap
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         97357824         98721792          1363968
0001         98721792         90693632         -8028160
0002         90693632         90693632                0
0003         90693632         90693632                0
0004         90693632         90693632                0
0005         90693632         90693632                0
0006         90693632         90693632                0
0007         90693632         90693632                0
0008         90693632         90693632                0
0009         90693632         90693632                0
.

Returning a HashMap
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         90693632         90693632                0
0001         90693632         90693632                0
0002         90693632         90693632                0
0003         90693632         90570752          -122880
0004         90570752         90570752                0


0005         90570752         90570752                0
0006         90570752         90570752                0
0007         90570752         90570752                0
0008         90570752         90570752                0
0009         90570752         90570752                0

Notes

  • I already run gc.collect() before measuring "after" memory
  • I already clear the identifier before measuring "after" memory with del result

Backtrace

No response

Your operating system and version

Linux pop-os 6.2.6-76060206-generic #202303130630168547333822.04~995127e SMP PREEMPT_DYNAMIC Tue M x86_64 x86_64 x86_64 GNU/Linux

Your Python version (python --version)

3.10.6

Your Rust version (rustc --version)

rustc 1.70.0 (90c541806 2023-05-31)

Your PyO3 version

0.19.0

How did you install python? Did you use a virtualenv?

Python installed with apt, and reproducer run inside a venv. These are the contents of the venv (already provided in requirements_dev.txt in the zip bundle above):

$ pip list
Package        Version
-------------- -------
exceptiongroup 1.1.1
iniconfig      2.0.0
maturin        1.1.0
packaging      23.1
pip            23.1.2
pluggy         1.2.0
psutil         5.9.5
pyo3mem        0.1.0
pytest         7.4.0
setuptools     59.6.0
tomli          2.0.1
wheel          0.40.0

Additional Info

I have extensive experience with Python and I can help provide any other assistance you might want.

@adamreichold
Copy link
Member

@cjrh Could you verify that #3286 does indeed fix the issue? Thank you.

@cjrh
Copy link
Author

cjrh commented Jul 2, 2023

@adamreichold The change certainly has an impact that removes the net new memory on every single call, however (on commit 1a0c9be ):

Returning a Vec
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         25804800         27017216          1212416
0001         27017216         27131904           114688
0002         27131904         27131904                0
0003         27131904         27131904                0
0004         27131904         27131904                0
0005         27131904         27131904                0
0006         27131904         27131904                0
0007         27131904         27131904                0
0008         27131904         27131904                0
0009         27131904         27131904                0
.

Returning a BTreeSet
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         27131904         29315072          2183168
0001         29315072         35606528          6291456
0002         35606528         35606528                0
0003         35606528         35606528                0
0004         35606528         35606528                0
0005         35606528         35606528                0
0006         35606528         35606528                0
0007         35606528         35606528                0
0008         35606528         35606528                0
0009         35606528         35606528                0
.

Returning a HashSet
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         35606528         35672064            65536
0001         35672064         35672064                0
0002         35672064         35205120          -466944
0003         35205120         35205120                0
0004         35205120         35205120                0
0005         35205120         35672064           466944
0006         35672064         35205120          -466944
0007         35205120         35205120                0
0008         35205120         35205120                0
0009         35205120         35205120                0
.

Returning a BTreeMap
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         35205120         35672064           466944
0001         35672064         40181760          4509696
0002         40181760         40181760                0
0003         40181760         40181760                0
0004         40181760         40181760                0
0005         40181760         40181760                0
0006         40181760         40181760                0
0007         40181760         40181760                0
0008         40181760         40181760                0
0009         40181760         40181760                0
.

Returning a HashMap
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         40181760         40226816            45056
0001         40226816         40226816                0
0002         40226816         40226816                0
0003         40226816         40226816                0
0004         40226816         40226816                0
0005         40226816         40226816                0
0006         40226816         40226816                0
0007         40226816         40226816                0
0008         40226816         40226816                0
0009         40226816         40226816                0

There is still an overall net growth that each function adds the first time it is called. Do you have an intuition about what that might be? The ~ 6 MB that the BTreeSet test adds, and the ~ 4.5 MB that the BTreeMap test adds both persist until the process terminates.

@cjrh
Copy link
Author

cjrh commented Jul 2, 2023

For convenience, here is my test pulled out of the zip I attached:

from contextlib import contextmanager
import gc
import pytest
import psutil
import pyo3mem


@contextmanager
def memlog(i: int):
    """Tool to repeatedly execute a block of code"""
    p = psutil.Process()
    m0 = p.memory_info().rss
    yield
    gc.collect()
    m1 = p.memory_info().rss
    print(f"{i:0>4} {m0:>16d} {m1:>16d} {(m1 - m0):>16d}")


@pytest.mark.parametrize('func,label,expected_type', [
    (pyo3mem.demo1, "Vec", list),
    (pyo3mem.demo2, "BTreeSet", set),
    (pyo3mem.demo3, "HashSet", set),
    (pyo3mem.demo4, "BTreeMap", dict),
    (pyo3mem.demo5, "HashMap", dict),
])
def test_mem(func, label, expected_type):
    print(f"\n\nReturning a {label}")
    print(
        f'{"iter":>4} {"mem before (b)":>16} {"mem after (b)":>16} {"leaked (bytes)":>16}'
    )
    n = 10
    for i in range(n):
        with memlog(i):
            result = func()
            assert type(result) is expected_type
            del result

It seems unlikely that e.g. the MB is being added to one of Python's long-lived arenas. I'll increase the test data size to see whether a larger allocation triggers an actual memory deallocation when the reference counts drop. In my test code, I have only one reference to the allocated data, which is result above, and I decrement the reference count with del result. So IMO that allocation should be getting deallocated pretty quickly. AFAIK there are no reference cycles here right?

@cjrh
Copy link
Author

cjrh commented Jul 2, 2023

Ok good news. I added a zero to the length of the test data returned by the various pyfunction functions (0..1000000) and now it seems clear that deallocation does happen:

Returning a Vec
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         25800704         26537984           737280
0001         26537984         26652672           114688
0002         26652672         26652672                0
0003         26652672         26652672                0
0004         26652672         26652672                0
0005         26652672         26652672                0
0006         26652672         26652672                0
0007         26652672         26652672                0
0008         26652672         26652672                0
0009         26652672         26652672                0
.

Returning a BTreeSet
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         26652672         46292992         19640320
0001         46292992         27598848        -18694144
0002         27598848         27590656            -8192
0003         27590656         27590656                0
0004         27590656         27590656                0
0005         27590656         27590656                0
0006         27590656         27521024           -69632
0007         27521024         27521024                0
0008         27521024         27521024                0
0009         27521024         27488256           -32768
.

Returning a HashSet
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         27488256         59592704         32104448
0001         59592704         27598848        -31993856
0002         27598848         27598848                0
0003         27598848         27598848                0
0004         27598848         27598848                0
0005         27598848         27598848                0
0006         27598848         27598848                0
0007         27598848         27598848                0
0008         27598848         27598848                0
0009         27598848         27598848                0
.

Returning a BTreeMap
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         27598848         27598848                0
0001         27598848         48418816         20819968
0002         48418816         48418816                0
0003         48418816         48418816                0
0004         48418816         48418816                0
0005         48418816         48418816                0
0006         48418816         48418816                0
0007         48418816         48418816                0
0008         48418816         48418816                0
0009         48418816         48418816                0
.

Returning a HashMap
iter   mem before (b)    mem after (b)   leaked (bytes)
0000         48418816         69263360         20844544
0001         69263360         27598848        -41664512
0002         27598848         69263360         41664512
0003         69263360         27598848        -41664512
0004         27598848         69259264         41660416
0005         69259264         27594752        -41664512
0006         27594752         69259264         41664512
0007         69259264         27574272        -41684992
0008         27574272         69238784         41664512
0009         69238784         27574272        -41664512

Takes a while but once we get to ~ 69 MB heap then deallocations back to the OS start to happen. We finish up pretty reliably on ~ 27.5 MB which is where we started (~ 25.8 MB).

Looks good 👍🏼

@cjrh
Copy link
Author

cjrh commented Jul 2, 2023

Would really appreciate a backport and a quick release into 0.18.4 🙏🏼

@adamreichold
Copy link
Member

Would really appreciate a backport and a quick release into 0.18.4 🙏🏼

Out of curiosity, why 0.18.x instead of 0.19.x which is what we would usually support?

@cjrh
Copy link
Author

cjrh commented Jul 2, 2023

Even happier if you can do the 0.19 branch, but I just assumed that would take longer because other stuff would be included so it'd be a bigger release process overall. 0.19.x is fine by me if that is also quick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants