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..338c12dd192 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 @@ -409,15 +411,35 @@ cdef class CoercionModel_cache_maps(CoercionModel): sage: cm = sage.structure.element.get_coercion_model() sage: maps, actions = cm.get_cache() - Now lets see what happens when we do a binary operations with + Now let us 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 + + 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 + sage: print right_morphism_ref None We can see that it coerces the left operand from an integer to a @@ -490,7 +512,7 @@ cdef class CoercionModel_cache_maps(CoercionModel): The function _test_exception_stack is executing the following code:: try: - raise TypeError, "just a test" + raise TypeError("just a test") except TypeError: cm._record_exception() """ @@ -518,7 +540,7 @@ cdef class CoercionModel_cache_maps(CoercionModel): ['Traceback (most recent call last):\n File "sage/structure/coerce.pyx", line ...TypeError: just a test'] """ try: - raise TypeError, "just a test" + raise TypeError("just a test") except TypeError: self._record_exception() @@ -758,7 +780,7 @@ cdef class CoercionModel_cache_maps(CoercionModel): all.append("Coercion on right operand via") all.append(y_mor) if res is not None and res is not y_mor.codomain(): - raise RuntimeError, ("BUG in coercion model: codomains not equal!", x_mor, y_mor) + raise RuntimeError("BUG in coercion model: codomains not equal!", x_mor, y_mor) res = y_mor.codomain() all.append("Arithmetic performed after coercions.") if op is div and isinstance(res, Parent): @@ -1045,7 +1067,7 @@ cdef class CoercionModel_cache_maps(CoercionModel): # We should really include the underlying error. # This causes so much headache. - raise TypeError, arith_error_message(x,y,op) + raise TypeError(arith_error_message(x,y,op)) cpdef canonical_coercion(self, x, y): r""" @@ -1265,33 +1287,76 @@ 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: gc.collect() #random + 852 + 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): @@ -1334,14 +1399,14 @@ cdef class CoercionModel_cache_maps(CoercionModel): if connecting is not None: R_map = R_map * connecting if R_map.domain() is not R: - raise RuntimeError, ("BUG in coercion model, left domain must be original parent", R, R_map) + raise RuntimeError("BUG in coercion model, left domain must be original parent", R, R_map) if S_map is not None and S_map.domain() is not S: if fix: connecting = S_map.domain()._internal_coerce_map_from(S) if connecting is not None: S_map = S_map * connecting if S_map.domain() is not S: - raise RuntimeError, ("BUG in coercion model, right domain must be original parent", S, S_map) + raise RuntimeError("BUG in coercion model, right domain must be original parent", S, S_map) # Make sure the codomains are correct if R_map.codomain() is not S_map.codomain(): if fix: @@ -1353,7 +1418,7 @@ cdef class CoercionModel_cache_maps(CoercionModel): if connecting is not None: R_map = connecting * R_map if R_map.codomain() is not S_map.codomain(): - raise RuntimeError, ("BUG in coercion model, codomains must be identical", R_map, S_map) + raise RuntimeError("BUG in coercion model, codomains must be identical", R_map, S_map) if isinstance(R_map, IdentityMorphism): R_map = None elif isinstance(S_map, IdentityMorphism): @@ -1431,9 +1496,9 @@ cdef class CoercionModel_cache_maps(CoercionModel): coerce_R = Z._internal_coerce_map_from(R) coerce_S = Z._internal_coerce_map_from(S) if coerce_R is None: - raise TypeError, "No coercion from %s to pushout %s" % (R, Z) + raise TypeError("No coercion from %s to pushout %s" % (R, Z)) if coerce_S is None: - raise TypeError, "No coercion from %s to pushout %s" % (S, Z) + raise TypeError("No coercion from %s to pushout %s" % (S, Z)) return coerce_R, coerce_S except Exception: self._record_exception() @@ -1716,5 +1781,3 @@ Original elements %r (parent %s) and %r (parent %s) and maps %s %r"""%( x_elt, y_elt, parent_c(x_elt), parent_c(y_elt), x, parent_c(x), y, parent_c(y), type(x_map), x_map, type(y_map), y_map) - -