Opened 4 years ago
Closed 4 years ago
#20268 closed defect (fixed)
memory leak in Polynomial.__call__
Reported by:  vdelecroix  Owned by:  

Priority:  blocker  Milestone:  sageduplicate/invalid/wontfix 
Component:  performance  Keywords:  
Cc:  nbruin, jdemeyer, vbraun  Merged in:  
Authors:  Reviewers:  Vincent Delecroix  
Report Upstream:  N/A  Work issues:  
Branch:  u/vdelecroix/20268 (Commits)  Commit:  4f15c250d5efa6e6d4a6e1cba1830cdf72519c34 
Dependencies:  Stopgaps: 
Description (last modified by )
The following cause a memory leak
sage: R = PolynomialRing(ZZ, 'x', implementation='NTL') sage: x = R.gen() sage: p = x**2  1 sage: while True: a = p(1)
See the following reports
We backport the fix provided in this commit.
Change History (13)
comment:1 Changed 4 years ago by
 Branch set to u/vdelecroix/20268
 Commit set to 2a2e0a112a7bfb1bd2d91d5f344d4ba10e4db58d
 Description modified (diff)
 Status changed from new to needs_review
comment:2 Changed 4 years ago by
I propose to deal with this in #20192 instead.
In any case, if you want the fix to be applied, you need to increase the Cython version number.
comment:3 Changed 4 years ago by
 Status changed from needs_review to needs_work
comment:4 Changed 4 years ago by
 Commit changed from 2a2e0a112a7bfb1bd2d91d5f344d4ba10e4db58d to 4f15c250d5efa6e6d4a6e1cba1830cdf72519c34
Branch pushed to git repo; I updated commit sha1. New commits:
4f15c25  Trac 20268: bump cython version

comment:5 Changed 4 years ago by
 Status changed from needs_work to needs_review
Since the stable cython release is about to be delivered, I guess it would be better to use #20192...
comment:6 Changed 4 years ago by
Robert Bradshaw says that Cython 0.24 will be delivered "Likely in the next week"
comment:7 Changed 4 years ago by
 Dependencies set to #20192
 Status changed from needs_review to needs_work
I would like to add a doctest for that. Do you think that the following is strong enough
sage: p = polygen(ZZ) sage: import resource sage: mem0 = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss sage: for _ in range(10000): ....: t = Polynomial.__call__(p, 1) sage: assert resource.getrusage(resource.RUSAGE_SELF).ru_maxrss < mem0 + 1000
If not, do you have a better idea?
comment:8 followup: ↓ 9 Changed 4 years ago by
Perhaps staying closer to python's own memory management routines and pick an example that produces stuff that is tracked by gc:
sage: import gc sage: p=polygen(ZZ) sage: gc.collect() 0 sage: N=len(gc.get_objects()) sage: for _ in range(10000): t = Polynomial.__call__(p,p) sage: assert len(gc.get_objects())N < 1000
comment:9 in reply to: ↑ 8 ; followup: ↓ 10 Changed 4 years ago by
Replying to nbruin:
Perhaps staying closer to python's own memory management routines and pick an example that produces stuff that is tracked by gc:
sage: import gc sage: p=polygen(ZZ) sage: gc.collect() 0 sage: N=len(gc.get_objects()) sage: for _ in range(10000): t = Polynomial.__call__(p,p) sage: assert len(gc.get_objects())N < 1000
It will not work. The leak was not seen at Python level.
comment:10 in reply to: ↑ 9 Changed 4 years ago by
Replying to vdelecroix:
It will not work. The leak was not seen at Python level.
Did you try the example? The leak causes dangling objects on the python heap. However, gc.get_objects()
will only find tracked objects, and tuples with too simple ingredients aren't tracked. However, when you put sufficiently complicated objects in there (such as p
) then it is tracked. So the test does work. So should the rusage of course, modulo unpredictable memory allocation strategies, or unusual circumstances that cause the rss to remain small in the presence of large memory usage.
comment:11 Changed 4 years ago by
 Dependencies #20192 deleted
comment:12 Changed 4 years ago by
 Milestone changed from sage7.2 to sageduplicate/invalid/wontfix
 Reviewers set to Vincent Delecroix
 Status changed from needs_work to positive_review
I guess this is superseded by the Cython upgrade.
comment:13 Changed 4 years ago by
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Trac 20268: upstream patch for memory leak