RFR (S): 8013872: G1: HeapRegionSeq::shrink_by() has invalid assert

Bengt Rutisson bengt.rutisson at oracle.com
Mon May 6 20:24:14 UTC 2013


Hi everyone,

Could I have a couple of reviews for this change?
http://cr.openjdk.java.net/~brutisso/8013872/webrev.00/

Background:
The method HeapRegionSeq::shrink_by() has an invalid assert. It wants to 
assert that we leave at least one region, but it triggers too early so 
if we want to shrink down to one single region we hit the assert.

When I started fixing the assert I thought the code was unnecessarily 
complicated. The method G1CollectedHeap::shrink_helper() uses both a 
number of regions and byte sizes to handle the shrinking. Both of these 
concepts are propagated down to HeapRegionSeq::shrink_by(), but that is 
not really necessary. It is enough for HeapRegionSeq::shrink_by() to 
know about the number of regions. That simplifies the code quite a bit 
and removes the need to have asserts that check that the byte sizes and 
region counts match.

So, rather than just fixing or removing the failing assert I refactored 
the code a bit to reduce the need for several of the asserts. I also 
changed the increment/decrement_length() methods that were a bit strange.

I think the HeapRegionSeq::expand_by() method can be simplified in a 
similar way, but I would like to limit this change to the shrinking.

I've tested the changes with JPRT and by running SPECjbb2005 with 
-XX:+PrintAdaptiveSizePolicy enabled to see that I get some heap shrinking.

The JTreg test that I am adding only fails on fastdebug or debug builds 
before my fix since it is an assert that is failing. Unfortunately there 
is no way to control that this test should only be run on fastdebug or 
debug builds.

Thanks,
Bengt




More information about the hotspot-gc-dev mailing list