diff --git a/src/sage/categories/fields.py b/src/sage/categories/fields.py index 95b83089269..054b1d194d6 100644 --- a/src/sage/categories/fields.py +++ b/src/sage/categories/fields.py @@ -97,11 +97,12 @@ def __contains__(self, x): sage: _ = gc.collect() sage: n = len([X for X in gc.get_objects() if isinstance(X, sage.rings.finite_rings.integer_mod_ring.IntegerModRing_generic)]) sage: for i in prime_range(100): - ... R = ZZ.quotient(i) - ... t = R in Fields() + ....: R = ZZ.quotient(i) + ....: t = R in Fields() + sage: del R sage: _ = gc.collect() sage: len([X for X in gc.get_objects() if isinstance(X, sage.rings.finite_rings.integer_mod_ring.IntegerModRing_generic)]) - n - 1 + 0 """ try: diff --git a/src/sage/categories/map.pxd b/src/sage/categories/map.pxd index 4d2b0d7fe63..33b994119d6 100644 --- a/src/sage/categories/map.pxd +++ b/src/sage/categories/map.pxd @@ -2,6 +2,8 @@ from sage.structure.parent cimport Parent from sage.structure.element cimport Element cdef class Map(Element): + cdef object __weakref__ + cdef public int _coerce_cost # a rough measure of the cost of using this morphism in the coercion system. # 10 by default, 100 if a DefaultCoercionMorphism, 10000 if inexact. diff --git a/src/sage/libs/singular/ring.pyx b/src/sage/libs/singular/ring.pyx index a535bfc638e..5a920139ea0 100644 --- a/src/sage/libs/singular/ring.pyx +++ b/src/sage/libs/singular/ring.pyx @@ -446,12 +446,14 @@ cdef ring *singular_ring_reference(ring *existing_ring) except NULL: INPUT: - - ``existing_ring`` -- an existing Singular ring. + - ``existing_ring`` -- a Singular ring. OUTPUT: - The same ring with its refcount increased. After calling this - function `n` times, you need to call :func:`singular_ring_delete` + The same ring with its refcount increased. If ``existing_ring`` + has not been refcounted yet, it will be after calling this function. + If initially ``existing_ring`` was refcounted once, then after + calling this function `n` times, you need to call :func:`singular_ring_delete` `n+1` times to actually deallocate the ring. EXAMPLE:: @@ -487,8 +489,7 @@ cdef ring *singular_ring_reference(ring *existing_ring) except NULL: if existing_ring==NULL: raise ValueError('singular_ring_reference(ring*) called with NULL pointer.') cdef object r = wrap_ring(existing_ring) - refcount = ring_refcount_dict.pop(r) - ring_refcount_dict[r] = refcount+1 + ring_refcount_dict[r] = ring_refcount_dict.get(r,0)+1 return existing_ring diff --git a/src/sage/rings/polynomial/plural.pyx b/src/sage/rings/polynomial/plural.pyx index 5983834ef69..6dc97afb04e 100644 --- a/src/sage/rings/polynomial/plural.pyx +++ b/src/sage/rings/polynomial/plural.pyx @@ -114,7 +114,7 @@ from sage.libs.singular.function cimport RingWrap from sage.libs.singular.polynomial cimport (singular_polynomial_call, singular_polynomial_cmp, singular_polynomial_add, singular_polynomial_sub, singular_polynomial_neg, singular_polynomial_pow, singular_polynomial_mul, singular_polynomial_rmul, singular_polynomial_deg, singular_polynomial_str_with_changed_varnames, singular_polynomial_latex, singular_polynomial_str, singular_polynomial_div_coeff) import sage.libs.singular.ring -from sage.libs.singular.ring cimport singular_ring_new, singular_ring_delete, wrap_ring +from sage.libs.singular.ring cimport singular_ring_new, singular_ring_delete, wrap_ring, singular_ring_reference from sage.libs.singular.singular cimport si2sa, sa2si, overflow_check @@ -327,7 +327,7 @@ cdef class NCPolynomialRing_plural(Ring): cdef RingWrap rw = ncalgebra(self._c, self._d, ring = P) # rw._output() - self._ring = rw._ring + self._ring = singular_ring_reference(rw._ring) self._ring.ShortOut = 0 self.__ngens = n diff --git a/src/sage/structure/coerce.pyx b/src/sage/structure/coerce.pyx index 97743825804..381784d710c 100644 --- a/src/sage/structure/coerce.pyx +++ b/src/sage/structure/coerce.pyx @@ -81,6 +81,8 @@ import operator cdef dict operator_dict = operator.__dict__ from operator import add, sub, mul, div, truediv, iadd, isub, imul, idiv +from cpython.weakref cimport PyWeakref_GET_OBJECT, PyWeakref_NewRef + from sage_object cimport SageObject from sage.categories.map cimport Map import sage.categories.morphism @@ -412,12 +414,32 @@ cdef class CoercionModel_cache_maps(CoercionModel): Now lets see what happens when we do a binary operations with an integer and a rational:: - sage: left_morphism, right_morphism = maps[ZZ, QQ] - sage: print copy(left_morphism) + sage: left_morphism_ref, right_morphism_ref = maps[ZZ, QQ] + + Note that by :trac:`14058` the coercion model only stores a weak + reference to the coercion maps in this case:: + + sage: left_morphism_ref + + + Moreover, the weakly referenced coercion map uses only a weak + reference to the codomain:: + + sage: left_morphism_ref() + (map internal to coercion system -- copy before use) Natural morphism: From: Integer Ring To: Rational Field - sage: print right_morphism + + To get an actual valid map, we simply copy the weakly referenced + coercion map:: + + sage: print copy(left_morphism_ref()) + Natural morphism: + From: Integer Ring + To: Rational Field + sage: print right_morphism_ref None We can see that it coerces the left operand from an integer to a @@ -1265,33 +1287,74 @@ cdef class CoercionModel_cache_maps(CoercionModel): True sage: parent(w+v) is W True + + TESTS: + + We check that with :trac:`14058`, parents are still eligible for + garbage collection after being involved in binary operations:: + + sage: import gc + sage: T=type(GF(2)) + sage: N0=len(list(o for o in gc.get_objects() if type(o) is T)) + sage: L=[ZZ(1)+GF(p)(1) for p in prime_range(2,50)] + sage: N1=len(list(o for o in gc.get_objects() if type(o) is T)) + sage: print N1 > N0 + True + sage: del L + sage: gc.collect() #random + 3939 + sage: N2=len(list(o for o in gc.get_objects() if type(o) is T)) + sage: print N2-N0 + 0 + """ try: - return self._coercion_maps.get(R, S, None) - except KeyError: - homs = self.discover_coercion(R, S) - if 0: - # This breaks too many things that are going to change - # in the new coercion model anyways. - # COERCE TODO: Enable it then. - homs = self.verify_coercion_maps(R, S, homs) + refs = self._coercion_maps.get(R, S, None) + if refs is None: + return None + R_map_ref, S_map_ref = refs + if R_map_ref is None: + S_map = PyWeakref_GET_OBJECT(S_map_ref) + if S_map is not None: + return None, S_map + elif S_map_ref is None: + R_map = PyWeakref_GET_OBJECT(R_map_ref) + if R_map is not None: + return R_map, None else: - if homs is not None: - x_map, y_map = homs - if x_map is not None and not isinstance(x_map, Map): - raise RuntimeError, "BUG in coercion model: coerce_map_from must return a Map" - if y_map is not None and not isinstance(y_map, Map): - raise RuntimeError, "BUG in coercion model: coerce_map_from must return a Map" - if homs is None: - swap = None + R_map = PyWeakref_GET_OBJECT(R_map_ref) + S_map = PyWeakref_GET_OBJECT(S_map_ref) + if R_map is not None and S_map is not None: + return R_map, S_map + except KeyError: + pass + homs = self.discover_coercion(R, S) + if 0: + # This breaks too many things that are going to change + # in the new coercion model anyways. + # COERCE TODO: Enable it then. + homs = self.verify_coercion_maps(R, S, homs) + else: + if homs is not None: + x_map, y_map = homs + if x_map is not None and not isinstance(x_map, Map): + raise RuntimeError, "BUG in coercion model: coerce_map_from must return a Map" + if y_map is not None and not isinstance(y_map, Map): + raise RuntimeError, "BUG in coercion model: coerce_map_from must return a Map" + if homs is None: + refs = None + swap = None + else: + R_map, S_map = homs + R_map_ref = None if R_map is None else PyWeakref_NewRef(R_map, None) + S_map_ref = None if S_map is None else PyWeakref_NewRef(S_map, None) + refs = R_map_ref, S_map_ref + if R_map is None and isinstance(S, Parent) and (S).has_coerce_map_from(R): + swap = None, PyWeakref_NewRef((S).coerce_map_from(R), None) else: - R_map, S_map = homs - if R_map is None and isinstance(S, Parent) and (S).has_coerce_map_from(R): - swap = None, (S)._internal_coerce_map_from(R) - else: - swap = S_map, R_map - self._coercion_maps.set(R, S, None, homs) - self._coercion_maps.set(S, R, None, swap) + swap = S_map_ref, R_map_ref + self._coercion_maps.set(R, S, None, refs) + self._coercion_maps.set(S, R, None, swap) return homs cpdef verify_coercion_maps(self, R, S, homs, bint fix=False):