Opened 7 years ago

Closed 6 years ago

#16411 closed defect (duplicate)

Memory leak in polynomial evaluation

Reported by: SimonKing Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: commutative algebra Keywords: memory leak polynomial evaluation libsingular
Cc: leif, malb Merged in:
Authors: Reviewers: Simon King
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The following was reported at sage-support:

sage: C.<x,y,z> = GF(2)[]
sage: f = x^4+x*y^3+z^6
sage: a = f(1,0,0)
sage: get_memory_usage()
176.08984375
sage: for i in xrange(1000000):
....:     a = f(1,0,0)
....:
sage: get_memory_usage()
198.08984375
sage: for i in xrange(1000000):
....:    a = f(1,0,0)
....:
sage: get_memory_usage()
222.08984375

In the following I am citing Leif's comments from sage-devel:

In singular_polynomial_call(&res, self._poly, _ring, coerced_x, MPolynomial_libsingular_get_element) we have

struct  spolyrec
{
   poly      next;           // next needs to be the first field
   number    coef;           // and coef the second --- do not change
this !!!
   unsigned long exp[VARS];  // make sure that exp is aligned
};

where both next and coef are pointers, and VARS is usually zero, so exp is an "open-ended" array, such that the effective size of the struct varies.

The leak depends on the values (and the amount also on the field and the function). My impression is also that it appears whenever res!=NULL, i.e., the result is non-zero.

The code in singular_polynomial_call() (in src/sage/libs/singular/polynomial.pyx) explicitly prevents Singular from reclaiming the memory:

     ...
     ret[0] = res_id.m[0]

     from_id.m[0] = NULL
     res_id.m[0] = NULL

     id_Delete(&to_id, r)
     id_Delete(&from_id, r)
     id_Delete(&res_id, r)

     return 0

(from_id.m[0] was set to the input parameter p, so that's ok.)

Either it should make a garbage-collected copy of it (the result ret / ret[0]) instead, or the caller has to clean up afterwards.

The docstring is quite misleading w.r.t. 'ret', as only the address of a pointer to be changed is passed to the function, while the struct it later points to always gets allocated by the callee, not the caller.

Change History (6)

comment:1 follow-up: Changed 7 years ago by SimonKing

I am not sure if this is a problem to be reported upstream. Probably not, it seems like the culprit is on our side, in sage.libs.singular.polynomial.

comment:2 in reply to: ↑ 1 Changed 7 years ago by leif

Replying to SimonKing:

It seems like the culprit is on our side, in sage.libs.singular.polynomial.

Yep, that's my impression at least. If the caller should clean up, it's sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular.__call__() (and probably more).

We'd presumably have to change both functions though.

(I haven't looked at the modules in whole; perhaps there are more potential leaks. new_MP() looked suspicious to me as well, but isn't used / called in the given example.)

comment:3 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:4 Changed 6 years ago by SimonKing

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Reviewers set to Simon King
  • Status changed from new to needs_review

With #18905, I get

sage: C.<x,y,z> = GF(2)[]
sage: f = x^4+x*y^3+z^6
sage: a = f(1,0,0)
sage: get_memory_usage()
1028.44921875
sage: for i in xrange(1000000):
....:     a = f(1,0,0)
....:     
sage: get_memory_usage()
1028.44921875

So, I suggest to close it as a duplicate.

comment:5 Changed 6 years ago by SimonKing

  • Status changed from needs_review to positive_review

(duplicate of #18905)

comment:6 Changed 6 years ago by vbraun

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.