RFR: JDK-8077842 - Remove the level parameter passed around in GenCollectedHeap
Mikael Gerdin
mikael.gerdin at oracle.com
Wed Jun 17 12:21:28 UTC 2015
Jesper,
On 2015-06-17 00:33, Jesper Wilhelmsson wrote:
> Mikael, Kim,
>
> Thank you very much for reviewing this!
>
> I'd prefer not to scope YoungGen and OldGen within GenCollectedHeap to
> make the code more readable. Cleaned it up and fixed an include that
> made some noise when building without precompiled headers.
>
> http://cr.openjdk.java.net/~jwilhelm/8077842/webrev.07/
> http://cr.openjdk.java.net/~jwilhelm/8077842/webrev.07.incremental/
Looks good to me, Reviewed.
/Mikael
>
> Thanks,
> /Jesper
>
>
> Kim Barrett skrev den 13/6/15 01:34:
>> On Jun 12, 2015, at 4:36 AM, Mikael Gerdin <mikael.gerdin at oracle.com>
>> wrote:
>>>
>>>> Oh.. I thought about your comments and found myself moving the enum to
>>>> GenCollectedHeap. This is the class that knows about the young and old
>>>> generations.
>>>
>>> Oh, ok :)
>>>
>>>>
>>>> In some kind of object oriented way, I don't think that a generation
>>>> should (have to) know it's type, or even that there are different types
>>>> of generations.
>>>
>>> I'm not sure I entirely agree with that statement, it seems to me
>>> that this is similar to the "old" way of having a bunch of
>>> generations floating around with only a "level" and no idea about how
>>> everything ties together. This was nice in theory but in fact all the
>>> generations are only ever able to function as an oldest or youngest
>>> generation and everything would break down if that was changed. I
>>> think it's just dishonest to not admit that a
>>> ConcurrentMarkSweepGeneration is an "old" generation and that a
>>> DefNewGeneration is a "young" generation.
>>> Nevertheless I think your change is a step in the right direction and
>>> I won't argue for changing this.
>>
>> I find myself continuing to waffle about this. But I agree it’s ok in
>> its current form, at least for now. If we eventually decide this
>> specific little bit does need to be changed, we can deal with it then.
>>
>>>> The new version is here:
>>>> http://cr.openjdk.java.net/~jwilhelm/8077842/webrev.06/
>>>> http://cr.openjdk.java.net/~jwilhelm/8077842/webrev.06.incremental/
>>>
>>> In genCollectedHeap.[ch]pp when you refer to the members of the
>>> GenerationType enum you don't need to scope them with GenCollectedHeap::
>>
>> But being explicit about the scope can make it easier to grep for them.
>>
>> Looks good.
>>
More information about the hotspot-gc-dev
mailing list