bad memory leak with exponentiation #4639

sagetrac-bober · 2008-11-27T19:06:08Z

I think that the following example speaks for itself. (This was on an x86, 32 bit, running Ubuntu.)

Also, I believe that these examples had no problems in sage 3.0.2.

bober@bober:~/math/tests$ sage
----------------------------------------------------------------------
| Sage Version 3.2, Release Date: 2008-11-20                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------

sage: get_memory_usage()
114.5546875
sage: for z in xrange(10000):
   ...:     a = 3^i
   ...:     
sage: get_memory_usage()
121.4375
sage: for z in xrange(10000):
    a = 3^CC.0
   ...:     
sage: get_memory_usage()
128.96484375
sage: for z in xrange(10000):
    a = 3.0^CC.0
   ...:     
sage: get_memory_usage()
187.36328125
sage: var('t')
t
sage: for z in xrange(10000):
    a = 3.0^t
   ....:     
sage: get_memory_usage()
231.4609375
sage: #But, integer^integer is OK:
sage: for z in xrange(10000):
    a = 3^3
   ....:     
sage: get_memory_usage
<function get_memory_usage at 0x8415f0c>
sage: get_memory_usage()
231.58984375
sage: for z in xrange(10000):
    a = 3^3
   ....:     
sage: get_memory_usage()
231.58984375
sage: for z in xrange(10000):
    a = 3.0^CC.0
   ....:     
saget_memory_usage()
290.1640625
sage: for z in xrange(10000):
    a = CC.0^CC.0
   ....:     
sage: get_memory_usage()
290.1640625

CC: @robertwb

Component: basic arithmetic

Issue created by migration from https://trac.sagemath.org/ticket/4639

The text was updated successfully, but these errors were encountered:

sagetrac-mabshoff · 2008-11-27T19:23:22Z

comment:1

This is indeed a grave bug.

Cheers,

Michael

sagetrac-mabshoff · 2008-11-28T02:31:09Z

comment:2

Hmm,

Burcin and I have been looking at code he has been writing and there seems to be a caching issue with the coercion system. Now, I don't want to outright blame coercion here without any hard evidence, but the timing (3.0.2 to now) suggests that it could be involved here. So let's add robertwb to the CC and see if he can come up with anything. I will collect some evidence and hopefully someone else will take over.

Cheers,

Michael

sagetrac-mabshoff · 2008-11-28T02:40:00Z

comment:3

Burcin about his code, not the problem reported here:

[6:36pm] burcin: hmm.. I think I found the problem..
[6:36pm] burcin: still vague though
[6:36pm] mabshoff: burcin: I CCed RobertWB on that existing leak ticket with the exponentiation and hopefully he will just post a one line patch.
[6:36pm] mabshoff: Ok 
[6:36pm] burcin: I was being a good coercion user and called .coerce()
[6:36pm] burcin: if I just use the __call__ on the parent, things work much much faster
[6:36pm] mabshoff: And it doesn't leak?
[6:37pm] burcin: I am guessing that the leak also goes away.. because this example ate more than 2gb memory with the leak
[6:37pm] mabshoff: yeah

burcin · 2008-11-28T02:51:49Z

comment:4

Here is my leak:

sage: F = GF(13)
sage: get_memory_usage()
708.02734375
sage: for _ in xrange(10000):
....:     t = F.coerce(F(234234))
....:     
sage: get_memory_usage()
728.15234375
sage: for _ in xrange(100000):
    t = F.coerce(F(234234))
....:     
sage: get_memory_usage()
932.3125
sage: for _ in xrange(100000):
    t = F.coerce(F(234234))
....:     
sage: get_memory_usage()
1136.35546875

sagetrac-mabshoff · 2008-11-28T21:35:49Z

comment:5

This ticket might also cause #4631.

Cheers,

Michael

robertwb · 2008-12-02T12:04:38Z

comment:7

Here's the bug:

sage: sage: get_memory_usage()
'465M'
sage: for _ in xrange(10000): t = Hom(QQ, QQ)
....: 
sage: sage: get_memory_usage()
'476M'
sage: Hom(QQ, QQ) is Hom(QQ, QQ)
False

sagetrac-mabshoff · 2008-12-02T15:49:34Z

comment:8

Hi Robert,

do you want me to valgrind here or are you already on top of this?

Cheers,

Michael

robertwb · 2008-12-02T19:50:02Z

comment:9

I think I can dig further, this is just as far as I got last night. It's probably a caching, not malloc, issue.

robertwb · 2008-12-02T22:25:18Z

Attachment: 4639-coerce-leak.patch.gz

this fixes the leak in coerce, not in exponentiation

robertwb · 2008-12-02T22:25:40Z

comment:10

Better caching for homsets fixes the leak for coercion.

sagetrac-mabshoff · 2008-12-02T22:43:51Z

comment:12

Maybe we should move the problem with exponentiation to another ticket if it proves difficult to fix so we can get the coercion fix in. I am doctesting this now, but I will hold on a merge until we have gotten #3623 and #4276 in.

Cheers,

Michael

sagetrac-mabshoff · 2008-12-03T00:05:29Z

comment:13

The patch RobertWB posted does pass doctesting.

Cheers,

Michael

sagetrac-mabshoff · 2008-12-03T01:16:00Z

comment:14

#4683 might be a dupe of this ticket.

Cheers,

Michael

williamstein · 2008-12-06T22:33:04Z

comment:15

I've read the patch and give it a positive review. But the patch doesn't resolve the ticket, as Robert points out. So after applying the patch, I guess you have to open a new ticket? Or???

Anyway -- don't just randomly close this ticket after applying the patch...

sagetrac-mabshoff · 2008-12-08T11:26:01Z

comment:16

I have deleted the patch and moved it to its own ticket at #4740 since it does not fix the original problem reported.

Cheers,

Michael

robertwb · 2008-12-14T05:51:51Z

comment:17

An interesting test point:

class TestParent(Parent):
    def __init__(self):
        self._populate_coercion_lists_()
    def _has_coerce_map_from_(self, X):
        return True
    def _element_constructor_(self, x):
        raise TypeError

Now this leaks

%python

from sage.all import get_memory_usage

def test(R, S):
    mor = S.convert_map_from(R)
    print get_memory_usage()
    for i in range(10000):
        try:
            mor = S.convert_map_from(R)
            a = mor._call_(R.gen())
        except:
            pass
    print get_memory_usage()

But this doesn't

%cython

from sage.all import get_memory_usage

def test(R, S):
    mor = S.convert_map_from(R)
    print get_memory_usage()
    for i in range(10000):
        try:
            mor = S.convert_map_from(R)
            a = mor._call_(R.gen())
        except:
            pass
    print get_memory_usage()

The only difference here is Python vs. Cython (leaking in the Python case).

robertwb · 2008-12-14T05:55:52Z

comment:18

And running

test(TestParent(), ZZ['x']) # python
638M
640M

test(TestParent(), ZZ['x']) # cython
640M
640M

It feels like something is getting cached in an exception.

robertwb · 2008-12-14T06:30:52Z

comment:19

I am almost sure the above is what's going wrong, and it gets invoked by call.

guppy gives a total of 40420 bytes on these runs... and nothing new after repeated runs (despite get_memory_usage going up and up). However, the memory still goes up. Mabshoff, could you valgrind this and see if anything suspicious comes up? Perhaps up the loop to 100000, or diff the run from Python with that from Cython, to make it more clear where the error is...

sagetrac-mabshoff · 2008-12-14T06:33:26Z

comment:20

Will do. Any news on the pickle failure for #4740?

Cheers,

Michael

robertwb · 2008-12-14T09:01:29Z

comment:21

This is a Cython error, not a coercion error. When something is returned from within a try block, it appears the (cached) exception is not released.

%cython
def foo():
    try:
        return None
    except:
        pass

%python
def test():
    print get_memory_usage()
    for i in range(100000):
        try:
            foo()
            raise TypeError
        except TypeError:
            pass
    print get_memory_usage()

Now test() will leak.

robertwb · 2008-12-14T11:28:24Z

comment:22

Install cython-0.10.3.spkg at http://sage.math.washington.edu/home/robertwb/cython/ , which contains a fix to http://trac.cython.org/cython_trac/ticket/162 and I think is the underlying cause here (and probably elsewhere).

sagetrac-mabshoff · 2008-12-14T15:53:39Z

comment:23

The new Cython.spkg reduces the leak significantly:
Without:

sage: get_memory_usage()
417.98828125
sage: for z in xrange(10000):
....:     a = 3.0^CC.0
....:     
sage: get_memory_usage()
509.26171875

With:

sage: get_memory_usage()
416.19140625
sage: 
sage: for z in xrange(10000):
....:     a = 3.0^CC.0
....:     
sage: get_memory_usage()
437.97265625

But unfortunately it doesn't fix it completely:

sage: get_memory_usage()
416.19140625
sage: 
sage: for z in xrange(10000):
....:     a = 3.0^CC.0
....:     
sage: get_memory_usage()
437.97265625
sage: for z in xrange(10000):
    a = 3.0^CC.0
....:     
sage: get_memory_usage()
459.99609375

Consequently I will split the Cython.spkg from this ticket.

Cheers,

Michael

sagetrac-mabshoff · 2008-12-14T17:41:59Z

comment:24

For the record: The cython.spkg upgrade to 0.10.3 is #4798.

Cheers,

Michael

robertwb · 2008-12-15T18:30:57Z

comment:25

This is the bug that will never die... I'll keep looking at that last case.

sagetrac-mabshoff · 2008-12-15T18:43:17Z

comment:26

If there ever was a blocker for 3.2.2 :)

Cheers,

Michael

simon-king-jena · 2008-12-16T13:16:29Z

comment:27

Another data point.

I took the example from #4683 (Michael, it was argued that #4683 is a duplicate).

sage: get_memory_usage()
704.5390625
sage: v = [CDF(i)^2 for n in range(50000)]
sage: get_memory_usage()
803.67578125
...
sage: get_memory_usage()
1093.203125
sage: del v
sage: get_memory_usage()
1093.203125
sage: v = [CDF(i)^2 for n in range(50000)]
sage: get_memory_usage()
1183.86328125
sage: del v
sage: get_memory_usage()
1183.86328125
sage: v=1
sage: get_memory_usage()
1183.86328125
...
sage: v = [CDF(i)^2 for n in range(50000)]
sage: get_memory_usage()
1279.55078125
sage: w = deepcopy(v)
sage: get_memory_usage()
1295.18359375
sage: del w
sage: get_memory_usage()
1295.18359375

Therefore it seems to me that the problem is not exponentiation but a failure in deallocation.

robertwb · 2008-12-16T20:29:23Z

comment:28

Attachment: 4639-parent-memleak.patch.gz

I believe I have located and resolved the issue. After tracing through all kinds of coercion and homset code, and reading tons of Cython code looking for memory leaks, it turns out that this was caused by all Parents ever created being cached due to old debugging code. (Wow, wish I'd looked there first! :)

Some old, unnecessary code has been removed from the generic __call__ method as well.

sagetrac-mabshoff · 2008-12-16T20:35:59Z

comment:29

Nice! I am testing away at the moment :)

Cheers,

Michael

sagetrac-mabshoff · 2008-12-16T20:38:31Z

comment:30

This does indeed fix the issue for me:

----------------------------------------------------------------------
| Sage Version 3.2.2.rc0, Release Date: 2008-12-15                   |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: get_memory_usage()
416.21484375
sage: for z in xrange(10000):
....:     a = 3.0^CC.0
....:     
sage: get_memory_usage()
416.21484375
sage: for z in xrange(10000):
    a = 3.0^CC.0
....:     
sage: get_memory_usage()
416.21484375

Doctesting now ...

Cheers,

Michael

sagetrac-mabshoff · 2008-12-17T03:28:13Z

comment:31

Positive review for 4639-parent-memleak.patch - w00t

Cheers,

Michael

sagetrac-mabshoff · 2008-12-17T14:01:23Z

comment:32

Merged in Sage 3.2.2.rc1

sagetrac-bober mannequin added c: basic arithmetic labels Nov 27, 2008

sagetrac-mabshoff mannequin added p: blocker / 1 and removed p: critical / 2 labels Nov 27, 2008

sagetrac-mabshoff mannequin added this to the sage-3.2.1 milestone Nov 27, 2008

sagetrac-mabshoff mannequin modified the milestones: sage-3.2.2, sage-3.2.1 Nov 28, 2008

sagetrac-mabshoff mannequin added the s: needs review label Dec 2, 2008

williamstein added s: positive review and removed s: needs review labels Dec 6, 2008

sagetrac-mabshoff mannequin modified the milestones: sage-3.4, sage-3.2.2 Dec 15, 2008

sagetrac-mabshoff mannequin added s: needs review and removed s: positive review labels Dec 16, 2008

sagetrac-mabshoff mannequin added s: positive review and removed s: needs review labels Dec 17, 2008

sagetrac-mabshoff mannequin removed the s: positive review label Dec 17, 2008

sagetrac-mabshoff mannequin closed this as completed Dec 17, 2008

This was referenced Dec 8, 2008

avoid creating unneeded homsets when coercing #4740

Closed

Update Cython to 0.10.2.p0 #4798

Closed

robertwb mentioned this issue Dec 17, 2008

update Cython to 0.10.3 #4819

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bad memory leak with exponentiation #4639

bad memory leak with exponentiation #4639

sagetrac-bober mannequin commented Nov 27, 2008

sagetrac-mabshoff mannequin commented Nov 27, 2008

sagetrac-mabshoff mannequin commented Nov 28, 2008

sagetrac-mabshoff mannequin commented Nov 28, 2008

burcin commented Nov 28, 2008

sagetrac-mabshoff mannequin commented Nov 28, 2008

robertwb commented Dec 2, 2008

sagetrac-mabshoff mannequin commented Dec 2, 2008

robertwb commented Dec 2, 2008

robertwb commented Dec 2, 2008

robertwb commented Dec 2, 2008

sagetrac-mabshoff mannequin commented Dec 2, 2008

sagetrac-mabshoff mannequin commented Dec 3, 2008

sagetrac-mabshoff mannequin commented Dec 3, 2008

williamstein commented Dec 6, 2008

sagetrac-mabshoff mannequin commented Dec 8, 2008

robertwb commented Dec 14, 2008

robertwb commented Dec 14, 2008

robertwb commented Dec 14, 2008

sagetrac-mabshoff mannequin commented Dec 14, 2008

robertwb commented Dec 14, 2008

robertwb commented Dec 14, 2008

sagetrac-mabshoff mannequin commented Dec 14, 2008

sagetrac-mabshoff mannequin commented Dec 14, 2008

robertwb commented Dec 15, 2008

sagetrac-mabshoff mannequin commented Dec 15, 2008

simon-king-jena commented Dec 16, 2008

robertwb commented Dec 16, 2008

sagetrac-mabshoff mannequin commented Dec 16, 2008

sagetrac-mabshoff mannequin commented Dec 16, 2008

sagetrac-mabshoff mannequin commented Dec 17, 2008

sagetrac-mabshoff mannequin commented Dec 17, 2008

bad memory leak with exponentiation #4639

bad memory leak with exponentiation #4639

Comments

sagetrac-bober mannequin commented Nov 27, 2008

sagetrac-mabshoff mannequin commented Nov 27, 2008

sagetrac-mabshoff mannequin commented Nov 28, 2008

sagetrac-mabshoff mannequin commented Nov 28, 2008

burcin commented Nov 28, 2008

sagetrac-mabshoff mannequin commented Nov 28, 2008

robertwb commented Dec 2, 2008

sagetrac-mabshoff mannequin commented Dec 2, 2008

robertwb commented Dec 2, 2008

robertwb commented Dec 2, 2008

robertwb commented Dec 2, 2008

sagetrac-mabshoff mannequin commented Dec 2, 2008

sagetrac-mabshoff mannequin commented Dec 3, 2008

sagetrac-mabshoff mannequin commented Dec 3, 2008

williamstein commented Dec 6, 2008

sagetrac-mabshoff mannequin commented Dec 8, 2008

robertwb commented Dec 14, 2008

robertwb commented Dec 14, 2008

robertwb commented Dec 14, 2008

sagetrac-mabshoff mannequin commented Dec 14, 2008

robertwb commented Dec 14, 2008

robertwb commented Dec 14, 2008

sagetrac-mabshoff mannequin commented Dec 14, 2008

sagetrac-mabshoff mannequin commented Dec 14, 2008

robertwb commented Dec 15, 2008

sagetrac-mabshoff mannequin commented Dec 15, 2008

simon-king-jena commented Dec 16, 2008

robertwb commented Dec 16, 2008

sagetrac-mabshoff mannequin commented Dec 16, 2008

sagetrac-mabshoff mannequin commented Dec 16, 2008

sagetrac-mabshoff mannequin commented Dec 17, 2008

sagetrac-mabshoff mannequin commented Dec 17, 2008