RFR (S): 8073052: Rename and clean up the allocation manager hierarchy in g1Allocator.?pp
Kim Barrett
kim.barrett at oracle.com
Thu Mar 5 21:40:45 UTC 2015
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.
------------------------------------------------------------------------------
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.
------------------------------------------------------------------------------
src/share/vm/gc_implementation/g1/g1Allocator.hpp
G1ParGCAllocator => PLABAllocator
Not sure I like the new name here. This class allocates G1PLAB's, but
the name implies allocation of "generic" PLAB's. I think
G1PLABAllocator would be more appropriate.
Similarly for G1DefaultPARGCAllocator => DefaultPLABAllocator.
But see comments below about G1PLAB naming.
------------------------------------------------------------------------------
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.
That should be a separate change from a bunch of renamings though.
Maybe that's already in the roadmap for these related changes, but I
didn't see it there.
------------------------------------------------------------------------------
More information about the hotspot-gc-dev
mailing list