| 8 | |
| 9 | __Problem analysis__ |
| 10 | |
| 11 | The quadratic field is created with a coerce embedding into `CLF`. At the same |
| 12 | time, this coerce embedding is stored in `CLF._coerce_from_hash`: |
| 13 | {{{ |
| 14 | sage: phi = CLF.coerce_map_from(Q) |
| 15 | sage: phi is Q.coerce_embedding() |
| 16 | True |
| 17 | sage: Q in CLF._introspect_coerce()['_coerce_from_hash'] |
| 18 | True |
| 19 | }}} |
| 20 | The "coerce_from_hash" is a `MonoDict`, hence, has only a weak reference to the key |
| 21 | (Q, in this case). However, there still is a ''strong'' reference from |
| 22 | CLF to the coerce map phi. And phi has a strong reference to its |
| 23 | domain, thus, to Q. Hence, the existence of CLF prevents garbage collection of |
| 24 | Q. |
| 25 | |
| 26 | And there is a second chain of strong references from CLF to Q: From CLF to |
| 27 | phi to the parent of phi (i.e., a homset) to the domain Q of this homset. |
| 28 | |
| 29 | __Suggested solution__ |
| 30 | |
| 31 | We can not turn the reference from CLF to phi into a weak reference, because |
| 32 | then even a strong reference to Q would not prevent phi from garbage |
| 33 | collection. Hence, we need to break the above mentioned reference chains in |
| 34 | two points. In the attached branch, maps generally keep a strong reference to |
| 35 | the codomain (this is important in composite maps and actions), but those used |
| 36 | ''in the coercion system'' (and ''only'' there!!) will only have a weak |
| 37 | reference to the domain, and they set the cdef `._parent` attribute to `None` |
| 38 | (hence, we also override `.parent()`, so that it reconstructs the homset if |
| 39 | the weak reference to the domain is still valid). |
| 40 | |
| 41 | To preserve the `domain()/codomain()` interface, I have removed the method |
| 42 | `domain()` and have replaced it by a cdef public attribute that will either |
| 43 | hold a weak reference (which returns the domain when called, hence, the |
| 44 | interface does not change) or a `ConstantFunction` (which should actually be |
| 45 | faster to call than a method). Since accessing a cdef attribute is still |
| 46 | faster, the cdef attribute `_codomain` is kept (since this will always be a |
| 47 | strong reference), but `_domain` has been removed. |
| 48 | |
| 49 | This "weakening of references" is done for the coercions found by |
| 50 | `discover_coerce_map_from()` stored into `_coerce_from_hash`. So, this mainly |
| 51 | happens for things done with `_coerce_map_from_()` and with composite |
| 52 | maps. Similarly for `_convert_from_hash`. |
| 53 | |
| 54 | Weakening is ''not'' used on the maps that are explicitly registered by |
| 55 | `.register_embedding()` and `.register_coercion()`. This is in order to |
| 56 | preserve the connectivity of the coercion graph. The `register_*` methods |
| 57 | are only used on selected maps, that are of particular importance for the |
| 58 | backtrack search in `discover_coerce_map_from()`. These ''strong'' |
| 59 | registrations do not propagate: Compositions of strongly registered |
| 60 | coercions found by `discover_coerce_map_from()` will be weakened. |
| 61 | |
| 62 | Since weakened maps should not be used outside of the coercion system, its |
| 63 | string representation shows a warning to replace them by a copy. The attached |
| 64 | branch implements copying of maps in some additional cases. |
| 65 | |
| 66 | `SchemeMorphism` can not inherit from `Morphism`, because of a bug with |
| 67 | multiple inheritance of a Python class from Cython extension classes. But once |
| 68 | this bug is fixed, we surely want to make `SchemeMorphism` inherit from |
| 69 | `Morphism`. This transition is prepared here. |
| 70 | |
| 71 | In any case, the commit messages should give a concise description of what has |
| 72 | been done. |
| 73 | |
| 74 | '''__Still TODO__''' |
| 75 | |
| 76 | Let the string representation of weakened maps point the user to the need of |
| 77 | creating a copy. |
| 78 | |
| 79 | '''__TODO in future tickets__''' |
| 80 | |
| 81 | - Provide a documentation of the use of weak references in coercion, and of |
| 82 | different ways of registering coercions, with their different impacts on |
| 83 | garbage collecion. |
| 84 | - Provide a version of `.register_coercion()` that weakens the coercion |
| 85 | map. It would hence have the same effect as returning a map by |
| 86 | `._coerce_map_from_()`, but of course `._coerce_map_from()` could not easily |
| 87 | be changed in an interactive session. |
| 88 | - provide copying for ''all'' kinds of maps. |
| 89 | |
| 90 | '''__Effects on the overall functioning of Sage__''' |
| 91 | |
| 92 | It is conceivable that some parts of Sage still suppose implicitly that stuff |
| 93 | cached with `UniqueRepresentation` is ''permanently'' cached, even though the |
| 94 | seemingly permanent cache was not more than a consequence of a memory leak in |
| 95 | the coercion system. With the attached branch, garbage collection of parent |
| 96 | structures will much more often become possible. Hence, code that relied on a |
| 97 | fake-permanent cache would now need to create the same parent repeatedly. |
| 98 | |
| 99 | I (Simon) have tested how many additional parent creations occur with the |
| 100 | attached branch when running `sage -t --all`. The findings are summarised in |
| 101 | comment:107: The number of additional parent creations increased by not more |
| 102 | than 1% for all but two parent classes (both related with tableaux). I also |
| 103 | found that the time to run the tests did not significantly increase. |
| 104 | |
| 105 | Jean-Pierre has occasionally stated that some of his computations have been |
| 106 | infeasible with the memory leak in the above example. I hope that his |
| 107 | computations will now succeed. |