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:

http://cr.openjdk.java.net/~brutisso/8034079/webrev.01/

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

http://cr.openjdk.java.net/~brutisso/8034079/webrev00.01.diff/

Thanks,
Bengt

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