devitocodes / devito

DSL and compiler framework for automated finite-differences and stencil computation
http://www.devitoproject.org
MIT License
562 stars 228 forks source link

Memory is not freed/keeps getting reallocated when creating SymbolicData objects #84

Closed vincepandolfo closed 8 years ago

vincepandolfo commented 8 years ago

As demonstrated by this snippet:

import gc
import numpy as np
import resource
from sympy import Eq

from devito.interfaces import DenseData
from devito.operator import Operator

def init(data):
    data = np.zeros(data.shape)

def test_memory(nx=1000, ny=1000):

    for i in range(10):
        gc.collect()

        u = DenseData(name='u', shape=(nx, ny), dtype=np.float64, space_order=2, initializer=init)

        eqn = Eq(u, u + 3)

        op = Operator(stencils=eqn, shape=(nx, ny), nt=10, spc_border=1)

        op.apply()

        print(resource.getrusage(resource.RUSAGE_SELF).ru_maxrss)

Output:

51084
64720
80364
88188
96020
103848
111692
119524
127348
135184

The memory for the u object keeps getting reallocated.

This is probably caused by the caching of Symbolic objects.

vincepandolfo commented 8 years ago

The issue seems to be that u keeps being added to the cache instead of reusing the cached instance

mlange05 commented 8 years ago

Ok, so this is partly by design, since the actual TimeData constructors get called explicitly by the user, while the cache is designed to catch SymPy's implicit re-instantiation. So, I think the right way to do this is to enforce clean deletion.

However, the clean deletion problem goes further than just the symbol cache, since the refcount for u goes up during the internal symbolic manipulation in the Operator/Propagator; it's 11 once we have built the Operator. Investigation continues...

vincepandolfo commented 8 years ago

I have a fix. It's ugly, but fixes the problem (and it is quite easy)

EDIT: fix in PR #85 EDIT: apparently that breaks a lot of other things. Closing that PR

mlange05 commented 8 years ago

Ok, I can confirm that this does indeed keep the memory volume down. I'm still not convinced we should cache in the natural constructor, and the fix is indeed very hacky, but it's a good start.

vincepandolfo commented 8 years ago

A possible solution would be to have a method clean that needs to be called explicitly by the user since there's no guarantee on if and when __del__ is called.

mlange05 commented 8 years ago

Yes, that sounds like a good intermediate solution. I'd love to get this fixed properly, but in the meantime this should work. Can you please implement it and submit a PR?

mlange05 commented 8 years ago

This has been addressed in PR #86. To recap, our symbolic objects are only deleted once the SymPy cache is cleared. We now provide an explicit method devito.clear_cache() that clears the sympy cache and cleans up all dead weak references in our own symbol cache to free up memory.