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