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

[Stefan Johansson]

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
>>>>
>>