RFR: 8042298 - Remove the names gen0 and gen1 from the GC code

[Jesper Wilhelmsson]

Thanks John!
/Jesper

John Coomes skrev 27/5/14 16:26:
> Jesper Wilhelmsson (jesper.wilhelmsson at oracle.com) wrote:
>> ...
>> So changes since last webrev is that all former usages of CollectGen0First is
>> now using ScavengeBeforeFullGC, and the removal of ScavengeBeforeFullGC is
>> reverted. There is also a check in arguments.cpp to set the correct default
>> value if not running ParallelGC. CollectGen0First and ScavengeBeforeFullGC had
>> different default values.
>>
>> An updated webrev is available here:
>>    http://cr.openjdk.java.net/~jwilhelm/8042298/webrev.3/
>
> This looks good to me.
>
> -John
>
>> Stefan Johansson skrev 16/5/14 13:45:
>>> Thanks Jesper,
>>>
>>> On 2014-05-16 02:10, Jesper Wilhelmsson wrote:
>>>> Hi,
>>>>
>>>> A new webrev is available here:
>>>> http://cr.openjdk.java.net/~jwilhelm/8042298/webrev.2/
>>>>
>>>> In addition to the last webrev the flags TraceGen0Time, TraceGen1Time and
>>>> CollectGen0First have been renamed to get rid of the Gen0/Gen1.
>>>>
>>>> Also, since CollectGen0First and ScavengeBeforeFullGC was doing the same thing
>>>> for different collectors, ScavengeBeforeFullGC has been merged with
>>>> CollectGen0First.
>>>>
>>> Looks good!
>>>
>>> Stefan
>>>> A CCC request has been filed for the flag changes.
>>>>
>>>> Thanks,
>>>> /Jesper
>>>>
>>>>
>>>> Stefan Johansson skrev 15/5/14 12:09:
>>>>> On 2014-05-14 23:21, Jesper Wilhelmsson wrote:
>>>>>> Hi Stefan,
>>>>>>
>>>>>> Thanks for looking at this! See comments inline.
>>>>>>
>>>>>> Stefan Johansson skrev 14/5/14 15:11:
>>>>>>> Hi Jesper,
>>>>>>>
>>>>>>> Thanks for doing this work. Here are some comments:
>>>>>>>
>>>>>>> src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp:
>>>>>>> * TraceGen0Time and TraceGen1Time should be changed to TraceYoungTime and
>>>>>>> TraceOldTime for consistency, and I guess that will need a CCC request.
>>>>>>>
>>>>>>> src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> * CollectGen0First -> CollectYoungFirst, same as above. Also used in
>>>>>>> tenuredGeneration.hpp.
>>>>>>>
>>>>>>> src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp
>>>>>>> * Comment starting on line 41 mentions TraceGen0Time and gen1.
>>>>>>
>>>>>> I agree that the names gen0 and gen1 should be completely removed, which
>>>>>> includes changing these flags too. Initially I wanted this change to be a
>>>>>> simple name change so that it would be easy to review it. Changing the flags
>>>>>> is borderline to be more than a name change, but I'm fine with including that
>>>>>> change.
>>>>>>
>>>>>> I'll file a CCC to change the flags.
>>>>>>
>>>>> Great, thanks.
>>>>>>> As an additional comment, I think that GenCollectedHeap::get_gen(int)
>>>>>>> should be
>>>>>>> broken up into two functions get_young() and get_old(). I get that this was
>>>>>>> left
>>>>>>> to a later patch to avoid having code/logic changes in this patch but I think
>>>>>>> the change is so little that it makes sense to do it in this patch. I would
>>>>>>> like
>>>>>>> to avoid having code like, even if it is for a brief period of time:
>>>>>>> Generation* young = gch->get_gen(0);
>>>>>>
>>>>>> This change is part of the larger removal of the generation array which is the
>>>>>> last part of this series of collector policy cleanups. Initially I was OK with
>>>>>> moving get_young() and get_old() to this change even though that would mean
>>>>>> that the change would grow considerably in size and wouldn't be a simple name
>>>>>> change anymore. After all the only reason I wanted to keep it small was to
>>>>>> make it easier for the reviewers :-)
>>>>>>
>>>>> That's good, thanks =)
>>>>>> However, when I started to look at it in more detail I realized that removing
>>>>>> get_gen(int) requires non-trivial changes in the serviceability agent which is
>>>>>> the only reason why the last change in this series isn't done yet.
>>>>>>
>>>>> I understand that you might want to do all the changes in the SA at once, and
>>>>> since this patch doesn't require you to change the SA I'm fine with leaving it
>>>>> as is.
>>>>>> It would be possible to introduce get_young() and get_old() and replace all
>>>>>> usages of get_gen(int) within the VM, but leave get_gen(int) for the SA to
>>>>>> use. I don't really like this approach, but if you prefer it over just
>>>>>> changing the names it's a possibility.
>>>>>>
>>>>> I'm fine with any of the approaches, none of them are perfect but both are good
>>>>> so I'll let you decide how you want to handle it.
>>>>>
>>>>> Thanks,
>>>>> Stefan
>>>>>> Thanks,
>>>>>> /Jesper
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Stefan
>>>>>>>
>>>>>>> On 2014-05-02 03:39, Jesper Wilhelmsson wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> This is a step towards removing the _generations array. The names gen0 and
>>>>>>>> gen1 are replaced with young and old to use a more standardized vocabulary.
>>>>>>>>
>>>>>>>> Webrev: http://cr.openjdk.java.net/~jwilhelm/8042298/webrev/
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8042298
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> /Jesper
>>>>>>>
>>>>>
>>>