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

[Sangheon Kim]

Hi Thomas,

On 03/05/2015 06:58 AM, 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

Change seems good.

Just minor comments.
- As Kim already pointed, just curious to have different indentation of 
access specifiers (protected:, public: ) in our source codes.
- Copyright year should be updated for 4 files.
   (g1CollectedHeap_ext.cpp, g1ParScanThreadState.cpp / hpp and 
heapRegionManager.cpp)

Thanks,
Sangheon

> Except for undoing the renaming of G1Allocator to G1AllocRegionManager,
> this change
> - removes some methods in G1CollectedHeap that were unused and do not
> have any implementation any more
> - moves methods from G1Allocator back to G1CollectedHeap that give
> information about the entire heap (e.g. related to _summary_bytes_used)
> as G1Allocator is about managing the current allocation regions only.
> - moves the new_heap_region() functionality back to G1CollectedHeap; it
> seems to have been moved only because this allowed easy override using
> the extension mechanism for closed additions. Its implementation has
> simply been moved to the g1CollectedHeap_ext.cpp file, achieving the
> same effect without cluttering the G1Allocator interface.
>
> Background of the upcoming changes:
>
> This is the second change that is part of the investigation of high
> fragmentation and waste during gc in g1 (JDK-8030849).
>
> This particular change forms the basis for the following further changes
> (in that order):
> JDK-8073013 - Add detailed information about PLAB memory usage
> JDK-8040162 - Avoid reallocating PLABs between GC phases in G1
> JDK-8067336 - Allow that PLAB allocations at the end of regions are
> flexible
> JDK-8067339 - PLAB reallocation might result in failure to allocate
> object in that recently allocated PLAB
> JDK-8067341 - Modify PLAB sizing algorithm to waste less
> JDK-8073317 - Move all region level allocation related things into
> AllocRegionManager
> JDK-8067433 - Keep waste at end of regions for further allocation during
> GC
>
> (There may be more intermediate steps depending on reviews).
> (During the investigation of JDK-8030849 even more issues were found,
> they might be added later, but typically they do not directly affect
> memory allocation performance during gc)
>
> That results in a substantial decrease in memory use during allocation,
> while setting higher PLAB sizes than the defaults used with
> -XX:-ResizePLAB. Applications that have a high number of mid-sized
> objects show an improvement in throughput of 5%, for others there is no
> measurable difference. The changes decrease the number of young gcs
> dramatically, allowing G1 to have more time for liveness analysis
> (marking). Which means that there is less possibility that g1 runs into
> a full gc.
>
> Since all of these CRs work in the same area I would like to avoid
> posting RFRs for all of them at the same time; however I put up a
> preliminary webrev up to and including JDK-8073317 at:
>
> http://cr.openjdk.java.net/~tschatzl/8073052/webrev.outlook
>
> Probably most interesting is the resulting interfaces in
> g1Allocator.hpp, at
> http://cr.openjdk.java.net/~tschatzl/8073052/webrev.outlook/src/share/vm/gc_implementation/g1/g1Allocator.hpp.html
> and how much related stuff (and friend declarations) can be removed in
> G1CollectedHeap:
> http://cr.openjdk.java.net/~tschatzl/8073052/webrev.outlook/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp.udiff.html
>
> Thanks,
>    Thomas
>
>