Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memleak in singular.pyx #11468

Closed
jpflori opened this issue Jun 13, 2011 · 18 comments
Closed

Memleak in singular.pyx #11468

jpflori opened this issue Jun 13, 2011 · 18 comments

Comments

@jpflori
Copy link

jpflori commented Jun 13, 2011

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.

CC: @jpflori @burcin

Component: memleak

Keywords: libsingular

Author: Jean-Pierre Flori

Reviewer: Mariah Lenox, Jonathan Bober

Merged: sage-4.7.2.alpha1

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

@jpflori jpflori added this to the sage-4.7.1 milestone Jun 13, 2011
@jpflori

This comment has been minimized.

@jpflori
Copy link
Author

jpflori commented Jun 15, 2011

Changed keywords from none to libsingular

@jpflori jpflori changed the title Memleak when using elliptic curves Memleak in multi_polynomial_libsingular.pyx Jun 15, 2011
@jpflori
Copy link
Author

jpflori commented Jun 15, 2011

comment:2

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

But it does not if called later.

@jpflori jpflori changed the title Memleak in multi_polynomial_libsingular.pyx Memleak in singular.pyx Jun 15, 2011
@jpflori

This comment has been minimized.

@jpflori
Copy link
Author

jpflori commented Jun 15, 2011

comment:4

I finally found the memleaks in different si2sa_* functions.

Potential fix provided.

@jpflori
Copy link
Author

jpflori commented Jun 15, 2011

Author: Jean-Pierre Flori

@sagetrac-mariah

This comment has been minimized.

@sagetrac-mariah
Copy link
Mannequin

sagetrac-mariah mannequin commented Jun 22, 2011

comment:8

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.

@jpflori
Copy link
Author

jpflori commented Jun 22, 2011

comment:9

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.

@sagetrac-mariah
Copy link
Mannequin

sagetrac-mariah mannequin commented Jun 22, 2011

comment:10

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.

@sagetrac-mariah
Copy link
Mannequin

sagetrac-mariah mannequin commented Jun 23, 2011

Reviewer: Mariah Lenox

@sagetrac-mariah
Copy link
Mannequin

sagetrac-mariah mannequin commented Jun 23, 2011

comment:11

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!

@jdemeyer
Copy link

comment:12

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

@jpflori
Copy link
Author

jpflori commented Jun 24, 2011

Attachment: trac_11468-memleaks_singular.patch.gz

New version with correct commit message and copyright.

@jpflori
Copy link
Author

jpflori commented Jun 24, 2011

comment:14

Just apply:

trac_11468-memleaks_singular.patch

@sagetrac-bober
Copy link
Mannequin

sagetrac-bober mannequin commented Jul 25, 2011

comment:15

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.

@sagetrac-bober
Copy link
Mannequin

sagetrac-bober mannequin commented Jul 25, 2011

Changed reviewer from Mariah Lenox to Mariah Lenox, Jonathan Bober

@jdemeyer
Copy link

Merged: sage-4.7.2.alpha1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants