RFR: 8047818: G1 HeapRegions can no longer be ContiguousSpaces

[Mikael Gerdin]

Jon,

On Tuesday 24 June 2014 07.26.52 Jon Masamitsu wrote:
> Mikael,
> 
> Did you consider creating a base class for ContiguousSpace and
> G1OffsetTableContigSpace that has a  _top but does not assume
> parsability?

I did consider it but I thought that the added complexity of having even more 
levels of inheritance were not worth the benefit of sharing the _top field. 
Especially since G1 attempts to hack around the semantics around the _top 
field with respect to concurrent access. See the disjunction I removed from 
allocate_impl and when G1 calls cas_allocate and allocate.

> 
> Could allocate_inner() have been called allocate_impl() as it is
> in ContiguousSpace?  I don't know what the "inner" in the name
> is telling me.

"inner" tries to signal that this function is internal and wrapped by other 
methods providing the external API. I don't have a particular naming 
preference here, if the other reviewers are fine with "impl" or don't have a 
preference I'm fine with changing it.

> 
> I've just started on the review so more to come.

Great!

Thanks
/Mikael

> 
> Jon
> 
> On 06/23/2014 07:26 AM, Mikael Gerdin wrote:
> > Hi!
> > 
> > When G1 is modified to unload classes without doing full collections the
> > old HeapRegions can contain unparseable objects. This makes
> > ContiguousSpace unsuitable as a base class for HeapRegion since it
> > assumes that all objects below _top are parseable.
> > 
> > Modify G1OffsetTableContigSpace to implement allocation with a separate
> > _top and reimplement some Space pure virtuals to make object iteration
> > work as expected.
> > 
> > This change is the last part of a set of 4 changes: 8047818, 8047819,
> > 8047820, 8047821 which are needed to refactor the HeapRegion class and
> > its superclasses in order to simplify the G1 class unloading change which
> > is coming. This change depends on the 19, 20 and 21 changes.
> > 
> > Bug:
> > https://bugs.openjdk.java.net/browse/JDK-8047818
> > Webrev:
> > http://cr.openjdk.java.net/~mgerdin/8047818/webrev/
> > 
> > Notes:
> > The moving of set_offset_range is due to an introduced circular dependency
> > between g1BlockOffsetTable.inline.hpp and heapRegion.inline.hpp
> > 
> > Thanks
> > /Mikael