RFR (M): 8034079: G1: Refactor the HeapRegionSet hierarchy

Bengt Rutisson bengt.rutisson at oracle.com
Tue Feb 11 10:50:29 UTC 2014

Hi again,

I fixed the serviceability agent (SA) and couple of minor things (in 
particular HeapRegionSetBase::_is_linked was not used). Here is an 
updated webrev:


Here is the diff compared to the previous version in case anybody has 
started looking at it:



On 2014-02-10 14:04, Bengt Rutisson wrote:
> Hi everyone,
> Could I have a couple of reviews for this change?
> http://cr.openjdk.java.net/~brutisso/8034079/webrev.00/
> It tries to simplify the structure of the HeapRegionSets. I need this 
> to be able to introduce new types of heap region collections.
> Some other cleanups are done as a consequence of this. But there are 
> probably still more cleanups to do. I hope that this is a step in the 
> right direction.
> Here's a short list of some of the changes:
> - Class hierarchy simplified
> No longer specific subclasses for all instances. Verification code has 
> been collected in the super classes and the MT safety checks are 
> aggregated instead.
> - Proxy sets replaced by HeapRegionSetCount
> The use of "proxy sets" was removed in favor of a light weight class 
> to keep track of length and capacity.
> - G1CollectedHeap::free_region_if_empty() was inlined
> - G1CollectedHeap::update_sets_after_freeing_regions() was split up 
> into three (or actually four) methods.
> - Removed the HRSPhaseSetter, which may soften the verification a bit 
> but not much.
> - The verification code also prints less information in some cases. 
> But instead it prints the relevant information. There is still more 
> cleanup that can be done to the messages from the asserts and 
> verifications.
> - Moved usage accounting up to avoid having to pass it around as much.
> - HeapRegionSetBase::total_used_bytes() was only used in asserts. Did 
> not seem worth keeping around.
> After these changes the file heapRegionSets.hpp (notice the s at the 
> end of the name) was empty so I removed it. However, the 
> heapRegionSets.cpp file is still present. I would like to move all the 
> code there in to heapRegionSet.cpp, but that would make the webrev 
> more difficult to follow. I'm still open to doing that if someone 
> would like me to do it.
> Thanks,
> Bengt

More information about the hotspot-gc-dev mailing list