RFR (M) 8075210: Refactor strong root processing in order to allow G1 to evolve separately from GenCollectedHeap

Mikael Gerdin mikael.gerdin at oracle.com
Thu Mar 19 08:43:12 UTC 2015


FYI,

On 2015-03-18 13:32, Thomas Schatzl wrote:
> Hi again,
>
> On Wed, 2015-03-18 at 13:28 +0100, Thomas Schatzl wrote:
>> Hi Mikael,
>>
>> On Tue, 2015-03-17 at 17:00 +0100, Mikael Gerdin wrote:
>>> On 2015-03-17 15:26, Thomas Schatzl wrote:
>>>> Hi Mikael,
>> [...]
>>>>
>>>> - I think the instantiation of G1RootsProcessor in
>>>> G1CollectedHeap::verify() should be scoped, i.e. it and the call to
>>>> process_all_roots() enclosed with braces.
>>>
>>> Fixed.
>>>
>>
>> Wrong indentation of the &blobsl parameter. I do not need to re-review
>> that.
>>
>> [...]
>>>
>>> Here's an incremental webrev incorporating your suggestions:
>>> http://cr.openjdk.java.net/~mgerdin/8075210/thomas-comments/webrev/
>>> It's based on
>>> http://cr.openjdk.java.net/~mgerdin/8075210/done_with_threads/webrev/
>>> which incorporates Eric's renaming request.
>>>
>>> New full webrev at:
>>> http://cr.openjdk.java.net/~mgerdin/8075210/full2/webrev/
>>
>> Looks good.
>
>
> Some last-minute comment if it is not still too late:
>
> - G1CollectedHeap::set_par_threads(uint t) can be removed. It only calls
> the inherited method anyway.

Actually, just removing the method causes a problem since 
G1CollectedHeap::set_par_threads() then hides 
SharedHeap::set_par_threads(uint)

"/opt/jprt/T/P1/160530.mgerdin/s/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp", 
line 1598: Warning: G1CollectedHeap::set_par_threads hides the virtual 
function SharedHeap::set_par_threads(unsigned).
1 Warning(s) detected.

I will add a
using SharedHeap::set_par_threads;
to G1CollectedHeap to explicitly import it into G1CollectedHeap.

I also plan to file a bug to enable this warning for gcc, I didn't hit 
it until it got to the Solaris build.

/Mikael

>
> Thanks,
>    Thomas
>
>



More information about the hotspot-gc-dev mailing list