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

[Thomas Schatzl]

Hi Kim,

  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)

Some comments below:

On Thu, 2015-03-05 at 16:40 -0500, Kim Barrett wrote:
> On Mar 5, 2015, at 9:58 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> > 
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8073052/webrev.01
> 
> ------------------------------------------------------------------------------
> Throughout
> 
> Adjusting indentation of access specifiers (e.g. "public:" &etc),
> changing from no indent to 1-space indent.  The code base appears to
> have a fairly random mix of no indent and 1-space indent for these.
> For example, src/share/vm has ~1900 1-space "public:" and ~1100
> 0-space, while in the g1 directory the mix is ~70 1-space and ~200
> 0-space.  I don't see any good reason to clutter this review with
> extraneous whitespace changes when there's no apparent consensus on
> what the whitespace usage ought to be.

I removed the whitespace changes, we should come to a consensus about
spacing of visibility identifiers.

> 
> ------------------------------------------------------------------------------  
> src/share/vm/gc_implementation/g1/g1Allocator.cpp
>  116 HeapWord* PLABAllocator::allocate_direct_or_new_plab(InCSetState dest,
>  117                                                        size_t word_sz,
>  118                                                        AllocationContext_t context) {
> 
> Update indentation of arguments.

Changed the name to G1PLABAllocator, which fixes this.

> ------------------------------------------------------------------------------  
> G1ParGCAllocBuffer => G1PLAB
> 
> That seems like a nice name change. However, it makes me wish
> ParGCAllocBuffer could be similarly shortened.
> 
> On the other hand, looking at G1PLAB (nee G1ParGCAllocBuffer), it
> doesn't really add much to its base class, and what it adds looks
> fairly generic and not G1-specific at all..  I wonder if G1PLAB could
> be eliminated altogether, merging the additional stuff into the base
> class, since it looks generally useful.

I created a new CR for these two concerns, renaming ParGCAllocBuffer and
trying to unify G1PLAB and ParGCAllocBuffer.

I agree that there may be interest in adding the retirement check
functionality for regular PLABs.

> That should be a separate change from a bunch of renamings though.

I added https://bugs.openjdk.java.net/browse/JDK-8074546

Thanks,
  Thomas