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:

Status badges

Description (last modified by mariah)

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)

trac_11468-memleaks_singular.patch (6.1 KB) - added by jpflori 10 years ago.
New version with correct commit message and copyright.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 10 years ago by jpflori

  • 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 jpflori

Calling gc.collect() just after the creation prevents the memory problem.

But it does not if called later.

comment:3 Changed 10 years ago by jpflori

  • 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 jpflori

  • Description modified (diff)

I finally found the memleaks in different si2sa_* functions.

Potential fix provided.

comment:5 Changed 10 years ago by jpflori

  • Authors set to Jean-Pierre Flori

comment:6 Changed 10 years ago by burcin

  • Cc burcin added

comment:7 Changed 10 years ago by mariah

  • Description modified (diff)

comment:8 Changed 10 years ago by mariah

  • 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: Changed 10 years ago by jpflori

  • 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.

See #11495 and #11521 for example.

comment:10 in reply to: ↑ 9 Changed 10 years ago by mariah

  • 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 mariah

  • 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 jdemeyer

  • 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

Changed 10 years ago by jpflori

New version with correct commit message and copyright.

comment:13 Changed 10 years ago by jpflori

  • Status changed from needs_work to needs_review

comment:14 Changed 10 years ago by jpflori

Just apply:

trac_11468-memleaks_singular.patch

comment:15 Changed 10 years ago by bober

  • 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 jdemeyer

  • Milestone changed from sage-4.7.1 to sage-4.7.2

comment:17 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.2.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.