| Title | AMS with default args never gets collected |
| Status | closed |
| Priority | essential |
| Assigned user | Gareth Rees |
| Organization | Ravenbrook |
| Description | Christian Schafmeister reports [1] that his root scanning function is not being called and indeed nothing gets collected. |
| Analysis | 1. The code uses the AMS pool class, but fails to pass a generation chain to the pool constructor. 2. This means that AMS allocates into generation 1 of the arena's default chain (see AMSInit [2], which "avoids the nursery of the default chain by default"). 3. There are two mechanisms by which this generation might be collected. First, via TraceStartCollectAll [3] which calls ChainCondemnAll [4] and second, via TracePoll [3] which calls ChainCondemnAuto [4]. 4. TraceStartCollectAll loops over all chains calling ChainCondemnAll, which condemns all segments in pools attached to generation zero of the chain. This mechanism relies on the assumption that if a pool has any collectable segments, then it is attached to generation zero of some chain. This assumption is unchecked and it is not true: with default args, an AMS pool gets attached to generation 1 of the default chain, and so will not be found. 5. TracePoll calls ChainDeferral for each chain, and if any returns a negative value (indicating that it is over capacity) then it collects the chain which is most over capacity. But ChainDeferral only compares allocation with capacity for generation zero of the chain. So if there is no allocation into generation zero of that chain, then the chain will never be collected (even if higher generations are over capacity). Note that this problem should have been identified earlier as it is also the cause of job003739. I worked around that problem by setting MPS_KEY_GEN to 0 in the test cases, when I should have investigated it properly and made sure I understood the cause. I can only apologise for the missed opportunity. Immediate workaround for Dr Schafmeister: pass the correct keyword arguments to the pool constructor! Test for problem (4): finaltest.c with the MPS_KEY_GEN keyword argument removed so that the pools get the default settings. Add extra test cases using the arena's default chain. For problem (4), alternate solution ideas: 4.1. In ChainCondemnAll, iterate over all generations. The trouble with this is that it is wasteful: we will end up attempting to condemn each segment multiple times (once for each generation that the pool is attached to). 4.2. We could work around that by storing, for each pool, a TraceSet which indicates for which traces we have condemned its segments. This would then be cleared for all pools at the end of TraceStartCollectAll. (Yuck!) 4.3. Change TraceStartCollectAll so that it ignores the chains, but instead loops over all the pools with AttrGC, condemning the segments in those pools. This is simplest, and has a pleasing parallelism with TraceCondemnZones (but it changes the meaning of the operation if there are pools that have AttrGC but are not attached to chains). Note that in this case we should add AVER(PoolHasAttr(pool, AttrGC)); to PoolGenInit so that this assumption gets checked. 4.4. Wait for the merge from the 2014-04-14/remember-time-2 branch, which will include a method for iterating over the segments of a generation. I plan to implement solution 4.3 temporarily; when the merge from the 2014-04-14/remember-time-2 branch lands we can use that. Test for problem (5): add another mode to finaltest that leaves the arena released and instead allocates objects to try to provoke a collection via TracePoll. Also, add tests for the arena default chain to amsss, and set the commit limit in to test that collections actually happen. For problem (5), alternate solution ideas: 5.1. Restore something like the old accounting hack, so that AMS and AWL account their allocation against the new size of generation zero of the chain they are allocating into. (Yuck! The whole point of the chain-zones work was to remove this ugly hack.) 5.2. In ChainDeferral, iterate over all generations of the chain and compute the appropriate measure, returning the smallest (that is, most over-capacity) value found. Change the logic in ChainCondemnAuto so that all generations up to and including the last that is over-capacity are condemned (instead of, as at present, condemnng all generations up to, but not including, the first generation that is in capacity). I plan to implement solution 5.2. |
| How found | customer |
| Evidence | [1] <https://info.ravenbrook.com/mail/2014/04/22/09-18-37/0/>[2] < http://www.ravenbrook.com/project/mps/master/code/poolams.c>[3] < http://www.ravenbrook.com/project/mps/master/code/trace.c>[4] < http://www.ravenbrook.com/project/mps/master/code/locus.c> |
| Test procedure | finaltest |
| Created by | Gareth Rees |
| Created on | 2014-04-22 14:17:22 |
| Last modified by | Gareth Rees |
| Last modified on | 2014-10-20 17:34:20 |
| History | 2014-04-22 GDR Created. |
| Change | Effect | Date | User | Description |
|---|---|---|---|---|
| 185741 | closed | 2014-04-22 17:53:47 | Gareth Rees | Fix bugs in condemn logic: 1. TraceStartCollectAll now condemns all segments in pools with AttrGC (not just pools attached to generation zero of some chain, as before). 2. ChainDeferral now looks at all generations in the chain, so that the chain is condemned if any generation's new size is greater than its capacity (not just generation zero, as before). 3. ChainCondemnAuto now condemns all generations up to and including the highest generation whose new size is greater than its capacity (rather than, as before, up to and excluding the lowest generation whose new size is lower than its capacity). Update finaltest.c so that it has a mode in which it allocates in generation 1 of a chain and with the arena released so that the above fixes are tested. Remove the MPS_KEY_GEN workarounds from awlut and awluthe as these are no longer needed. |