CRR: 7042285 and 7045330 : fixes and improvements to the HeapRegionSeq class (S/M)

Tony Printezis tony.printezis at oracle.com
Wed Jun 1 11:51:45 UTC 2011


Hi,

Could I please have a couple of code reviews for this change:

http://cr.openjdk.java.net/~tonyp/7045330.7042285/webrev.0/

The associated CRs are:

7042285: G1: native memory leak during humongous object allocation
7045330: G1: Simplify/fix the HeapRegionSeq class

The first is the fix for the memory leak when the heap is expanded / 
retracted aggressively that I opened for code review some time ago. We 
are currently not reclaiming HeapRegion instances when the heap is 
retracted, but we are creating new ones when the heap is expanded. But, 
in all honesty, the HeapRegionSeq class is a bit of a mess so I decided 
to expand the fix, cleanup that class, and introduce a couple of small 
performance improvements too (all these are covered by the second CR). 
Here's a list of the improvements (copied from the CR):

a) replace the _regions growable array with a standard C array (as we do 
not grow or shrink it so there's no point in using a growable array)
b) avoid de-allocating / re-allocating HeapRegion instances when the 
heap shrinks / grows (fix for 7042285)
c) introduce fast method to map address to HeapRegion via a "biased" 
array pointer
d) embed the _hrs object in G1CollectedHeap, instead of pointing to it 
via an indirection
e) assume that all the regions added to the HeapRegionSeq instance are 
in a contiguous address space
f) replace int's with size_t's for region indexes (part of 6804436)
g) remove unnecessary / unused methods
h) rename a couple of fields to give them a more reasonable name 
(_alloc_search_start and _seq_bottom)
i) fix iterate_from() to do the right thing and not always start from 
index 0 irrespective of the region passed to it
j) add a verification method to check the HeapRegionSeq assumptions
k) always call the wrappers for _hrs.iterate(), _hrs_length(), and 
_hrs.at() from G1CollectedHeap, not those methods directly.

In terms of volume of changes, the HeapRegionSeq class has been 
rewritten heavily. The rest of the changes should be straightforward 
(there are changes in G1CollectedHeap that reflect the slightly 
different API that HeapRegionSeq now exposes).

Performance-wise, I see a again modest (~2% or maybe a bit more) 
improvement in GC times on a couple of platforms.

Tony




More information about the hotspot-gc-dev mailing list