RFR (S): 8073052: Rename and clean up the allocation manager hierarchy in g1Allocator.?pp

[Thomas Schatzl]

Hi Stefan,

  thanks for the review.

There is a new webrev at
http://cr.openjdk.java.net/~tschatzl/8073052/webrev.2 (full)
http://cr.openjdk.java.net/~tschatzl/8073052/webrev.1_to_2 (diff)

Further comments inline:

On Fri, 2015-03-06 at 10:00 +0100, Stefan Johansson wrote:
> Hi Thomas,
> 
> On 2015-03-05 15:58, Thomas Schatzl wrote:
> > Hi all,
> >
> > On Wed, 2015-03-04 at 16:28 +0100, Thomas Schatzl wrote:
> >> Hi all,
> >>
> >>    can I have reviews for the following change that does some renamings
> >> in the allocation related class hierarchy, also adding a few lines of
> >> documentation.
> >>
> >> This change is intentionally limited to renames to keep it simple, in a
> >> subsequent patch there will be more cleanup changes, moving methods
> >> around.
> >> Also, if there is demand, I am open to rename the files to something
> >> different (suggestions?) in a follow-up change (to not confuse the
> >> webrev tool too much).
> >>
> >> CR:
> >> https://bugs.openjdk.java.net/browse/JDK-8073052
> >> Webrev:
> >> http://cr.openjdk.java.net/~tschatzl/8073052/webrev.00/
> >> Testing:
> > I talked with Stefan Johansson a bit about the change, with the result
> > that we keep the name of G1Allocator, since in the upcoming changes (see
> > below for more) that class is supposed to handle all allocations within
> > the current allocation regions.
> >
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8073052/webrev.01
> Generally change looks good, but I agree with Kim that PLABAllocator 
> could still use the G1 prefix for consistency. Some other comments:

Done.

> ---
> g1CollectedHeap.hpp:
> +  // Allocates a new heap region instance.
> +  virtual HeapRegion* new_heap_region(uint hrs_index, MemRegion mr);
> 
> No need to have this one virtual, no one extends G1CollectedHeap.

Done.

> ---
> vmStructs_g1.hpp:
> Since _summary_bytes_used have been moved in the native code this needs 
> to be reflected in the Java agent as well. You need to move the 
> functionality from G1Allocator.java to G1CollectedHeap.java.

Done.

Thanks,
  Thomas