RFR (M): 8199742: Clean up state flags in G1CollectorState

[sangheon.kim]

Hi Thomas,

On 03/27/2018 06:25 AM, Thomas Schatzl wrote:
> Hi Stefan,
>
> On Tue, 2018-03-27 at 14:58 +0200, Stefan Johansson wrote:
>> Hi Thomas,
>>
>> Thanks for this much needed cleanup :)
>>
>> On 2018-03-26 17:06, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     I would like to request reviews for this change that cleans up
>>> the flags in G1CollectorState, applying uniform naming, removing
>>> members that were basically temporary variables for a single
>>> method, and [...]
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8199742
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8199742/webrev
>> The change looks good. Just some minor things, gc_are_young and
>> during_im are used as local variables at some places. I think we
>> could change those to reflect the new better names as well.
> I grepped through the sources for the old names and changed the uses to
> something better (I hope).
>
> http://cr.openjdk.java.net/~tschatzl/8199742/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8199742/webrev.1 (full)
webrev.1 looks good to me.
I have some minor comments.

=======================
src/hotspot/share/gc/g1/g1CollectorState.hpp
41   // If initiate_conc_mark_if_possible() is set at the beginning of a
- _initiate_conc_mark_if_possible instead of 
initiate_conc_mark_if_possible() as other cases are using a variable name.

=======================
src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
1899   if 
(!G1CollectedHeap::heap()->collector_state()->mark_or_rebuild_in_progress()) 
{
- Pre-existing issue: _g1h instead of G1CollectedHeap::heap(). This 
should be commented on JDK-8151171 but I missed. :) Probably need an 
extra CR for G1CollectedHeap::heap() cleanup as other classes also would 
use it even though the class has a member of G1CollectedHeap. e.g. 
g1CollectionSet.cpp:260

I don't need an extra webrev for above comment(s) if you will fix.

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>