memory leak, possibly in crystals

[mantepse]

As noticed in #27057:

sage: import gc
sage: C = crystals.Letters("A5")
sage: for d in range(10,1,-1):
....:     T = crystals.TensorProduct(*([C]*d))
....:     hi = T.highest_weight_vectors()
....:     _ = gc.collect()
....:     _ = gc.collect()
....:     print get_memory_usage(), len(hi)
....:     
5321.24609375 9096
5322.49609375 2556
5322.74609375 756
5322.99609375 231
5322.99609375 76
5322.99609375 26
5322.99609375 10
5322.99609375 4
5322.99609375 2

sage: C = crystals.Letters(['A', 1])
sage: C._dummy = True
sage: del C
sage: import gc
sage: _ = gc.collect()
sage: C = crystals.Letters(['A', 1])
sage: C._dummy
True
sage: C = ShiftedPrimedTableaux([2,1], max_entry=2)
sage: C._dummy = True
sage: del C
sage: _ = gc.collect()
sage: C = ShiftedPrimedTableaux([2,1], max_entry=2)
sage: C._dummy
True

CC: @tscrim @slel

Component: memleak

Reviewer: Samuel Lelièvre

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

[mantepse]

comment:1

Actually, I think this is to be expected with UniqueRepresentation, no?

sage: C = SetPartitions()
sage: C._dummy = True
sage: del C
sage: C = SetPartitions()
sage: C._dummy
True

[jdemeyer]

comment:2

How is this a memory leak? Why is this a bug?

[nbruin]

comment:3

Replying to @jdemeyer:

How is this a memory leak? Why is this a bug?

It is a little suspicious because the UniqueRepresentation object C is deleted, so one would assume C should now be unreachable. Then gc.collect() is run, which should clean unreachable objects. So you'd expect after that to get a clean copy of C when it is recreated. But as you can see, it is not: it still has this _dummy attribute.

Testing creating and deleting these objects in a loop and then checking if any pile up on the heap using gc.get_objects doesn't give worrisome results, however. And when WeakRefs are involved it's actually possible to prevent gc from collecting everything in one swoop, so I agree there doesn't seem to be a memory leak.

But I think the report had some merit to it in that it pointed out suspicious behaviour that warrants a little further investigation (which found nothing).

[tscrim]

comment:4

Martin, it is not because of UniqueRepresentation. Although I am a little surprised that running gc.collect() a few times after deleting it is not leading to a new object being created:

sage: P = GF(3037000453)['x','y']
sage: P.dummy = True
sage: P.dummy
True
sage: del P
sage: gc.collect() # I ran this 5 times
sage: P = GF(3037000453)['x','y']
sage: P.dummy
...
AttributeError

Although in this example, it is not a UniqueRepresentation object, but I think it illustrates what should happen.

I am also wondering if this is related to #18426.

[tscrim]

comment:5

I do not seem to be able to actually free the memory in the loop mentioned.

Nils, can you remind me the code of how to check with gc.get_objects()? I can never seem to remember it.

[tscrim]

comment:6

I flopped the direction of the loop to illustrate the problem more. Maybe the get_memory_usage is not the right test...

[mantepse]

comment:7

Replying to @tscrim:

Martin, it is not because of UniqueRepresentation.

I only thought so because you can reproduce it also with other UniqueRepresentation instances. For example:

sage: C = SetPartitions()
sage: C.dummy = True
sage: del C
sage: gc.collect()
sage: C = SetPartitions()
sage: C.dummy
True

[nbruin]

comment:8

Replying to @tscrim:

I do not seem to be able to actually free the memory in the loop mentioned.

Nils, can you remind me the code of how to check with gc.get_objects()? I can never seem to remember it.

There's a pretty clean example on the ticket #18426 you referenced above and which you reported. That's where I copied the code from when I tried. I actually made a mistake. There does seem to be some leakage, but it seems capped, oddly enough; like there's an LRU cache in place or so.

import gc
from collections import Counter
gc.collect()
pre={id(a) for a in gc.get_objects()}
for n in [2..600]:
  r = ShiftedPrimedTableaux([n,1], max_entry=2)
  r._dummy= True
del r
gc.collect()
gc.collect()
gc.collect()

T=Counter(str(type(a)) for a in gc.get_objects() if id(a) not in pre)
[t for t in T.iteritems() if 'Primed' in t[0]]

For me on 8.6, this finds 128 objects. And that number remains constant if I increase the bound 600 above.

[tscrim]

comment:9

Replying to @nbruin:

Replying to @tscrim:

I do not seem to be able to actually free the memory in the loop mentioned.

Nils, can you remind me the code of how to check with gc.get_objects()? I can never seem to remember it.

There's a pretty clean example on the ticket #18426 you referenced above and which you reported.

facepalm

That's where I copied the code from when I tried. I actually made a mistake. There does seem to be some leakage, but it seems capped, oddly enough; like there's an LRU cache in place or so.

Running this variant

import gc
from collections import Counter
gc.collect()
pre={id(a) for a in gc.get_objects()}
for n in range(1000,2,-1):
    C = crystals.Letters(['A',n])
    T = tensor([C]*n)
    del C
    del T
gc.collect()
gc.collect()
gc.collect()

T=Counter(str(type(a)) for a in gc.get_objects() if id(a) not in pre)
[t for t in T.iteritems() if 'letters' in t[0] or 'tensor_product' in t[0]]

results in

[("<type 'sage.combinat.crystals.letters.Crystal_of_letters_type_A_element'>",
  624),
 ("<class 'sage.combinat.crystals.tensor_product.FullTensorProductOfRegularCrystals_with_category'>",
  32),
 ("<class 'sage.combinat.crystals.letters.ClassicalCrystalOfLetters_with_category'>",
  32)]
sage: sum(crystals.Letters(['A',n]).cardinality() for n in range(3,35))
624

So I agree, there seems to be some sort of LRU cache going on, but no real memory leak.

Actually, I have vague recollection of some sort of discussion about doing an LRU cache on sage-devel at some point, but I don't remember the outcome or details.

[mantepse]

comment:10

Replacing ShiftedPrimedTableaux([n,1], max_entry=2) with SetPartitions(n) I get <class 'sage.combinat.set_partition.SetPartitions_set_with_category'>": 128.

Perhaps the thread https://groups.google.com/forum/#!searchin/sage-devel/lru/sage-devel/q5uy_lI11jg/kRWKxvCImwEJ from long ago is relevant?

[nbruin]

comment:11

Oh boy. It would seem that #24954 is responsible for this:

unique_representation.py:1012

indeed it mentions a "128". So yes, there is no memory leak in the code, but after #24954 any test will indeed perceive a leak of something like 128 elements. In fact, with crystal.Letters there is apparently another implied construction, so we end up with only "64" elements.

This was exactly why I wasn't happy with #24954. It makes finding memory error that much harder. On the plus side: as this example shows, it does drive home the global nature of UniqueRepresentation elements a little more, and the fact that you shouldn't add or modify attributes on them.

[jdemeyer]

comment:12

See #24954 indeed.

[slel]

comment:13

Let us close this if nobody objects.

[slel]

Reviewer: Samuel Lelièvre