RFR (M): 8219747: Remove g1_ prefix to g1_remset and g1_policy members in G1CollectedHeap

[sangheon.kim at oracle.com]

Hi Thomas,

On 2/27/19 4:44 AM, Thomas Schatzl wrote:
> Hi,
>
>    thanks for your review.
>
> On Wed, 2019-02-27 at 11:41 +0100, Aleksey Shipilev wrote:
>> On 2/27/19 11:09 AM, Thomas Schatzl wrote:
>>>    can I have reviews for this renaming change that removes some
>>> "g1_" prefixes to some G1CollectedHeap members? They are simply
>>> unnecessary within the context of G1.
>>>
>>> There has also been a "g1_collector_policy" method - after looking
>>> at its uses I decided to remove it and add a specific whitebox
>>> support method for it to G1CollectedHeap instead.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8219747
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8219747/webrev/
>> Looks good. It is a bit confusing to see the rename is_hetero_heap ->
>> is_heap_heterogeneous in this
>> changeset that talks about g1_ prefix removal, though.
> I removed this and will file separately.
>
>> Nits:
>>
>> *) Indentation in g1CollectedHeap.cpp:
>>
>>   864   if (policy()->need_to_start_conc_mark("concurrent humongous
>> allocation",
>>   865                                            word_size)) {
>>
>> 4449   policy()->phase_times()-
>>> record_fast_reclaim_humongous_time_ms((os::elapsedTime() -
>> start_time) * 1000.0,
>> 4450
>> cl.humongous_objects_reclaimed());
>>
>> *) Not sure if you want to rename locals too, like in
>> g1ConcurrentMarkThread.cpp:
>>
>>   246   G1Policy* g1_policy = g1h->policy();
>>
> Fixed (also local names).
>
> http://cr.openjdk.java.net/~tschatzl/8219747/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8219747/webrev.1/ (full)
Webrev.1 looks good.

Just minor nits: only actual changes for hetero are reverted while 
copyright year isn't on webrev.1. Guessing '8219856: Spell out 
G1CollectedPolicy::is_hetero_heap' will be pushed together, I'm okay.

Thanks,
Sangheon


>
> Still compiles locally.
>
> Thanks,
>    Thomas
>
>