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