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

[Bengt Rutisson]

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
>
>