Opened 10 years ago
Closed 10 years ago
#11468 closed defect (fixed)
Memleak in singular.pyx
Reported by: | jpflori | Owned by: | rlm |
---|---|---|---|
Priority: | critical | Milestone: | sage-4.7.2 |
Component: | memleak | Keywords: | memleak, libsingular |
Cc: | jpflori, burcin | Merged in: | sage-4.7.2.alpha1 |
Authors: | Jean-Pierre Flori | Reviewers: | Mariah Lenox, Jonathan Bober |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Using the following piece of code makes the memory footprint of sage
grow indefinitely:
sage: K = GF(1<<50,'t') sage: R.<x,y> = PolynomialRing(K) sage: a = K.random_element() sage: while 1: ....: R(a) ....:
The memleak happens when different si2sa_* functions are called.
See http://groups.google.com/group/sage-support/browse_thread/thread/9a8e887df34a8e9a for further discussion.
Attachments (1)
Change History (18)
comment:1 Changed 10 years ago by
- Description modified (diff)
- Keywords libsingular added; elliptic curves removed
- Summary changed from Memleak when using elliptic curves to Memleak in multi_polynomial_libsingular.pyx
- Type changed from PLEASE CHANGE to defect
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
- Status changed from new to needs_review
- Summary changed from Memleak in multi_polynomial_libsingular.pyx to Memleak in singular.pyx
comment:4 Changed 10 years ago by
- Description modified (diff)
I finally found the memleaks in different si2sa_* functions.
Potential fix provided.
comment:5 Changed 10 years ago by
comment:6 Changed 10 years ago by
- Cc burcin added
comment:7 Changed 10 years ago by
- Description modified (diff)
comment:8 Changed 10 years ago by
- Status changed from needs_review to needs_work
The patch does not seem to fix the reported problem. I applied the patch to sage-4.7.1.alpha2, did 'sage -b', yet I still see memory increasing when I run the code in the ticket description.
comment:9 follow-up: ↓ 10 Changed 10 years ago by
- Status changed from needs_work to needs_info
I just retested it and it seems to fix the memleak for me:
no increase in memory footprint after 30 minutes.
Which code did you run ?
The one in the tickect description or the one on sage-support post ?
Because there are other memleaks involved when using EllipticCurve? class.
comment:10 in reply to: ↑ 9 Changed 10 years ago by
- Status changed from needs_info to needs_review
Replying to jpflori:
I just retested it and it seems to fix the memleak for me:
no increase in memory footprint after 30 minutes.
Which code did you run ?
The one in the tickect description or the one on sage-support post ?
I used the code in the ticket description.
I will try again.
comment:11 Changed 10 years ago by
- Reviewers set to Mariah Lenox
- Status changed from needs_review to positive_review
I must have forgotten to do 'sage -b' when I tried before. Apologies for the noise.
This time when I applied the patch (and did 'sage -b') the code in the ticket description ran for 30 CPU minutes without an increase in memory. I also did 'make testlong' and all tests passed.
Positive review!
comment:12 Changed 10 years ago by
- Status changed from positive_review to needs_work
The commit message
Trac 114666666x memleaks in singular.pyx
looks a bit odd...
Also, if you change the "copyright" message, at least make it such that it looks like http://sagemath.org/doc/developer/conventions.html#headings-of-sage-library-code-files
comment:13 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:14 Changed 10 years ago by
Just apply:
trac_11468-memleaks_singular.patch
comment:15 Changed 10 years ago by
- Reviewers changed from Mariah Lenox to Mariah Lenox, Jonathan Bober
- Status changed from needs_review to positive_review
This patch looks good to me. I don't get a leak with either the example or the code I was running that lead me to this ticket, and the changes that the patch makes seem simple and sensible.
comment:16 Changed 10 years ago by
- Milestone changed from sage-4.7.1 to sage-4.7.2
comment:17 Changed 10 years ago by
- Merged in set to sage-4.7.2.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
Calling gc.collect() just after the creation prevents the memory problem.
But it does not if called later.