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

[Jesper Wilhelmsson]

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.

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