RFR: 8058298: Separate heap region iterator claim values from the data structures iterated over

Marcus Larsson marcus.larsson at oracle.com
Mon Oct 6 12:17:43 UTC 2014


Updated the change according to Bengt's feedback, but left the friend 
declarations in place. Also, since ParallelGCThreads is not allowed to 
be 0 any longer [1], I simplified the claimer and removed the initialize 
function and the empty constructor.

New webrev:



[1]: https://bugs.openjdk.java.net/browse/JDK-8059527

On 06/10/14 13:37, Bengt Rutisson wrote:
> Hi Thomas,
> On 2014-10-06 13:26, Thomas Schatzl wrote:
>> Hi,
>> On Mon, 2014-10-06 at 13:08 +0200, Bengt Rutisson wrote:
>>> Hi Marcus,
>>> Nice work!
>>> It looks really good to me.
>>> Some minor comments:
>>> heapRegionManager.cpp
>> [...]
>>> Can we introduce a getter in G1CollectedHeap for this instead of adding
>>> two friend declarations?
>>> G1CollectedHeap::heap()->_hrm._allocated_heapregions_length
>> I somewhat prefer the friend declaration:
>> _hrm._allocated_heapregions_length is somewhat of a hack and is
>> preferably not available from outside, because potentially people start
>> using it for iteration over the heap again then ;)
>> What could be done to hide this is to create a factory method for the
>> HeapRegionClaimer in HeapRegionManager, or pass the HeapRegionClaimer
>> instance to a HeapRegionManager instance for initialization.
> I discussed this a bit with Marcus. I think creating a factory for the
> claimer or passing it to the manager for initialization seems like a
> step in the right direction. However, I think that would make it even
> more obvious that you really want an iterator and not just a claimer. ;)
> After some discussion I'm fine with keeping the current friend
> declarations. If we decide to introduce iterators we can clean this up
> then. It is probably not worth going half the way right now.
> Thanks,
> Bengt
>> Thanks,
>>    Thomas

More information about the hotspot-gc-dev mailing list