MPS issue job003737

TitlePool class attributes incorrect, unchecked
Assigned userGareth Rees
DescriptionSee design/type/ [1] for a description of pool class attributes.

Pool classes are supposed to have attributes INCR_RB if they require a read barrier and INCR_WB if they require a write barrier. INCR_WB is not set on any pool classes, but some pool classes do require write barriers; and neither INCR_RB nor INCR_WB is ever checked.

Some other attributes could be checked more systematically (or at all). See analysis.
AnalysisProblems with attributes:
1. There's no interface for checking them, so modules need to explicitly look up pool->class->attr which seems naughty. [I added PoolHasAddr]
2. Design contradicts itself ("attributes are only used for consistency checking ... it's no longer true that they are only used for consistency checking") [I removed this text, and documented what these attribute are actually used for]
3. MOVINGGC is used but undocumented. [added to design]
4. MOVINGGC must imply GC but that is not checked. [now checked in PoolClassCheck]
5. PM_NO_READ and PM_NO_WRITE are defined and documented, but not used. [removed from design]
6. SCAN should be checked systematically by PoolScan, not opportunistically in TraceQuantum. [moved check]
7. SCAN should be checked systematically in SegCheck (segments with ranks must belong to scannable pools), not opportunistically in TraceStart. [moved check]
8. BUF_RESERVE seems to be redundant with BUF [removed it]
9. BUF_ALLOC is defined and documented but not used [removed it]

Problems with pool use of attributes:
1. AMCZ inherits from AMC and so has to clear the SCAN and INCR_RB attributes. This indicates that the inheritance is the wrong way round: AMC should inherit from AMCZ, not vice versa. [swapped]
2. AMCZ resets grey and scan methods but forgets to do the same to the blacken method. This error would not occur if the inheritance were reversed. [swapped]
3. MRG sets the FREE attribute but does not provide a free method. [don't set it]
4. PoolClassMixInCollect sets INCR_RB but this isn't right: you only need a read barrier if there are references as well as collection. [don't set it]
5. AMS sets FMT but does not provide a walk method. [used PoolNoWalk]
How foundinspection
Evidence[1] <>
Created byGareth Rees
Created on2014-04-04 15:06:37
Last modified byGareth Rees
Last modified on2016-04-15 19:26:30
History2014-04-04 GDR Created.


Change Effect Date User Description
185231 closed 2014-04-04 17:05:08 Gareth Rees Tidy-up of attributes and pool classes:
* Bring design up to date.
* New function PoolHasAttr encapsulates attribute checking.
* Abstract classes are abstract and mustn't be checked.
* The dummy pool class in fotest needs a size.
* Abstract pool classes null out methods that they can't provide a generic implementation for, to force subclasses to provide one.
* New function PoolTrivFramePopPending provides a generic implementation of that method.
* Rename PoolNoFreeWalk to PoolTrivFreeWalk since it has NOOP rather than NOTREACHED.
* Check that AttrMOVINGGC implies AttrGC.
* Remove unimplemented attributes (BUF_RESERVE, BUF_ALLOC, INCR_RB, INCR_WB, PM)
* AMC now inherits from AMCZ instead of the other way round. This is simpler: AMC adds features to AMCZ rather than AMCZ taking features away (and not quite getting it right).
* Similarly, LO inherits from AbstractSegBufPoolClass + PoolClassMixInCollect so that it doesn't have to clear AttrSCAN and the scan methods.
* Fix bug in MFSCheck -- mustn't check unroundedUnitSize >= UNIT_MIN since small unit sizes are rounded up to UNIT_MIN.
* Don't see AttrFREE in MRG (since no free method is supplied).
* Check AttrSCAN systematically (in PoolScan and SegCheck) rather than opportunistically in TraceStart and TraceQuantum.