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

[Marcus Larsson]

Hi Thomas,


On 09/25/2014 01:08 PM, Thomas Schatzl wrote:
> Hi Marcus,
>
> On Fri, 2014-09-19 at 13:13 +0200, Marcus Larsson wrote:
>> Hi Thomas, thanks for reviewing!
>>
> [...]
>> New webrev:
>> http://cr.openjdk.java.net/~brutisso/webrev-8058298v2/
>>
>> Changes from previous webrev:
>> http://cr.openjdk.java.net/~brutisso/webrev-8058298v1-v2/
>>
>    thanks for the changes, look good.
>
> One final request :) Is it possible to make the HRClaimer of
> G1ParRemoveSelfForwardPtrsTask a member of the task like it is done for
> all the other tasks? Then we would also get rid of the parameterless
> constructor for HeapRegionClaimer.

I can do that. This alone will however not allow us to get rid of the 
parameterless constructor unfortunately.
Some tasks still conditionally use the HRClaimer 
(G1ParVerifyFinalCountTask, G1ParFinalCountTask for example),
initializing the HRClaimer in the task constructor only if 
use_parallel_gc_threads() is true.

> I saw that you are also working on "JDK-6979279: remove special-case
> code for ParallelGCThreads==0". With that change,
> G1CollectedHeap::use_parallel_gc_threads() would be always true,
> allowing to make HeapRegionClaimer::initialize() private.

Yes, together with that change it would be possible to remove the empty 
constructor and just keep
the initialization code in the other constructor, removing the need for 
the initialize function completely.
I'm however thinking the above patch could include those changes to the 
HRClaimer, since it is only possible
after making those changes anyway.

For now, I moved the claimer to the task as you requested.

New webrev:
http://cr.openjdk.java.net/~mlarsson/8058298/webrev.02/

Incremental webrev:
http://cr.openjdk.java.net/~mlarsson/8058298/webrev.01-02/


Thanks,
Marcus

> Thanks,
>    Thomas
>
>