RFR: 8047818: G1 HeapRegions can no longer be ContiguousSpaces

[Mikael Gerdin]

On Tuesday 24 June 2014 14.20.30 Thomas Schatzl wrote:
> Hi,
> 
> On Mon, 2014-06-23 at 16:26 +0200, 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
> 
>   a few minor nits:
> 
>  - in G1OffsetTableContigSpace::cas_allocate_inner(), the method should
> access _top directly per coding guidelines

I interpret this as a request to change to
  HeapWord* obj = _top;
Should I change other uses of top() as well?

I could only find 
https://wiki.openjdk.java.net/display/HotSpot/StyleGuide#StyleGuide-Accessors 
as a reference here. Do you interpret that as "only use public accessors if 
outside the class"?


There have been a few requests for renaming and changing the *allocate 
functions to be either exact copies of the ones in ContiguousSpace or even to 
break *allocate and _top into a separate class, how do you feel about this?

>  - just a note: _top should be declared volatile as it is used in the
> CAS, although the code is correct. However there is already an issue for
> that https://bugs.openjdk.java.net/browse/JDK-8033552, so I suggest
> postponing this.

Ok.

>  - extra newline after G1OffsetTableContigSpace::allocate_inner()
>  - extra newline after G1BlockOffsetSharedArray::set_offset_array()

I'll remove these.

/Mikael

> 
> Thanks,
>   Thomas