Opened 7 years ago
Last modified 16 months ago
#18426 new enhancement
Memory leaks in RootSystem
Reported by: | tscrim | Owned by: | sage-combinat |
---|---|---|---|
Priority: | critical | Milestone: | sage-6.7 |
Component: | memleak | Keywords: | memory leak |
Cc: | sage-combinat, nthiery, slelievre | Merged in: | |
Authors: | Travis Scrimshaw | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
RootSystem
is currently creating memory leaks in the following way:
Upon creating a root system, say of type B3, this line:
self.dual = RootSystem(self._cartan_type.dual(), as_dual_of=self);
causes the B3 root system to be used as a key in the UniqueRepresentation
of the C3 root system (I call it so, but note it is the dual of the B3, which will be different than the honest C3 root system), which is a strong reference that can never be deleted. Moreover, the B3 root system then holds a (strong) reference to the C3 root system, so the C3 root system never gets collected either.
Some data:
sage: from collections import Counter sage: import gc sage: gc.collect() 349 sage: pre = {id(a) for a in gc.get_objects()} sage: get_memory_usage() 8697.9453125 sage: for n in range(5, 3000): ....: RS = RootSystem(['A', n]) ....: sage: gc.collect() 106 sage: get_memory_usage() 8703.08984375 sage: post = Counter(str(type(a)) for a in gc.get_objects() if id(a) not in pre) sage: sorted([p for p in post.items() if p[1] > 2000]) [("<class 'dict'>", 6123), ("<class 'sage.combinat.root_system.root_system.RootSystem'>", 5985), ("<class 'sage.combinat.root_system.type_A.CartanType'>", 2991), ("<class 'tuple'>", 30460), ("<class 'weakref.KeyedRef'>", 9003)]
Change History (5)
comment:1 Changed 7 years ago by
- Description modified (diff)
comment:2 Changed 7 years ago by
- Description modified (diff)
Hi Travis!
I am sure there are plenty of other similar situations of cross-referencing parents (e.g. in SymmetricFunctions
). How critical do you think this specific situation is? I mean: do you foresee people creating thousands of different root systems in the same Sage session?
Cheers,
Nicolas
comment:3 Changed 7 years ago by
I'm not so sure about how frequent we get the cross-referencing parents with one of them being a key for a UniqueRepresentation
. Although it's hard to tell because there are known memory leaks with the category Algebras
that would keep parents alive.
For this particular situation, I doubt anyone will create thousands of root systems. What I'm a little worried about is all the different things a root system might keep alive and could pile up (like root lattices (and say you tested them over a large range of finite fields, but this might not be mathematically reasonable), Cartan types/matrices, etc.).
comment:4 Changed 7 years ago by
Questions:
- do you really need
RootSystem
to beUniqueRepresentation
(i.e., do these things really participate in the coercion framework)? I would think the "parent" component of aRootSystem
is only there because mathematically it looks like something that has elements, not as something whose elements will be interacting with elements from other parents. - do you really need
RootSystem
to hold any references to other root systems? Doesn't theUniqueRepresentation
cache provide fast enough access? - can you key the
UniqueRepresentation
via lower level information, e.g., a sequence of integers or so? By changing to aUniqueFactory
instead, you could still offer the same interface.
Especially because these things probably happen all over the place, I think it's really good that you're trying to see what kind of solution strategies work well. I'm sure your experience in this can be used in lots of other places. It's hard to predict what people will be doing with the code, so we should try to not make assumptions and unnecessarily restrict usage scenarios.
One possible workaround is to not accept a root system as input, but instead a boolean. Then when it creates the dual root system, and then explicitly sets the
dual
attribute of the dual root system toself
.