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

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Wed May 14 21:21:02 UTC 2014


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.

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

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.

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.

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
>



More information about the hotspot-gc-dev mailing list