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