search

manodeep / Corrfunc

⚡️⚡️⚡️Blazing fast correlation functions on the CPU.
https://corrfunc.readthedocs.io
MIT License
163 stars 50 forks source link

DDrppi bug memory leak #227

Closed Richard-Sti closed 3 years ago

Richard-Sti commented 3 years ago

General information

Issue description

Each Corrfunc.theory.DDrppi call appears to be leaking memory.

Expected behavior

Calling DDrppi should not leave any memory inaccessible.

Actual behavior

The amount of potentially leaked memory depends on the problem but is on order of O(100 KB) on each call. The extra allocated memory can be tracked to Corrfunc/theory/DDrppi.py:297.

In the minimal failing example if you keep running it the reserved memory will continuously increase until the program crashes.

What have you tried so far?

Tried explicitly deleting the variables and calling the Python garbage collector.

Minimal failing example

import numpy as np
from Corrfunc.theory import DDrppi

rpbins = np.logspace(np.log10(0.1), np.log10(25), 13)
x, y, z = [np.random.uniform(0, 400, 50000) for __ in range(3)]

while True:
    __ = DDrppi(1, 1, 60, rpbins, x, y, z)
lgarrison commented 3 years ago

Thanks for the report! I will try to reproduce it this week.

If you do copy_particles=False, does the leak persist? Obviously, it should not leak in either case, though!

Richard-Sti commented 3 years ago

Thanks! Yes, the leak persists even if I set copy_particles=False.

lgarrison commented 3 years ago

@Richard-Sti I think I found the leak, a Python reference leak to the results structure. I think it's present for all modules, but the rppi leak is the biggest because it's 2D. Could you try PR #229?

manodeep commented 3 years ago

That is fantastic!

@lgarrison The C-python extensions are proving to be annoying with the memory leaks - should we consider switching to something like cffi?

lgarrison commented 3 years ago

Yes! I'm a big fan of CFFI. I haven't used it at this scale, though!

Richard-Sti commented 3 years ago

@lgarrison I compiled PR #229, ran the minimal failing example and the leak looks solved now. Awesome, thanks!

lgarrison commented 3 years ago

Merged! Thanks @Richard-Sti. This will be part of the next release.

Richard-Sti commented 3 years ago

Great! Thanks for fixing it this quickly:)

shanquan-gui commented 2 years ago

@Richard-Sti How could you fix this issue? Could you try explain it a bit further or with a python code? I have the same issue, but I do not know how to fix it .

lgarrison commented 2 years ago

Hi @shanquan-gui , this issue was fixed in Corrfunc v2.4. If you're still having problems, feel free to open a new issue, and we'll be happy to help.