RFR: 8183542: Factor out serial GC specific code from GenCollectedHeap into its own subclass
Roman Kennke
rkennke at redhat.com
Wed Oct 25 08:07:53 UTC 2017
Hi Kim, hi Jini,
thank you both for your reviews!
I think I need a sponsor now. The final webrev (same as before plus
Reviewed-by line):
http://cr.openjdk.java.net/~rkennke/8183542/webrev.03/
<http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.03/>
Thanks, Roman
>> On Oct 19, 2017, at 12:39 PM, Roman Kennke <rkennke at redhat.com> wrote:
>>
>> Am 18.10.2017 um 22:41 schrieb Kim Barrett:
>>>> On Oct 18, 2017, at 4:04 PM, Roman Kennke <rkennke at redhat.com> wrote:
>>>>
>>>> Am 18.10.2017 um 20:41 schrieb Kim Barrett:
>>>>>> On Oct 18, 2017, at 8:08 AM, Roman Kennke <rkennke at redhat.com> wrote:
>>>>>> Differential webrev:
>>>>>> http://cr.openjdk.java.net/~rkennke/8183542/webrev.01.diff/ <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.01.diff/>
>>>>>>
>>>>>> Full webrev:
>>>>>> http://cr.openjdk.java.net/~rkennke/8183542/webrev.01/ <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.01/>
>>>>>>
>>>>>> Better now?
>>>>>>
>>>>>> Thanks, Roman
>>>>> Looks good.
>>>>>
>>>> Hi Kim,
>>>>
>>>> thanks for the review.
>>>>
>>>> I just fixed a bug caused by my similar CMSHeap extraction, and I think I need to do the same thing for SerialHeap too:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8189373
>>>>
>>>> This is the fix for the CMSHeap issue:
>>>>
>>>> http://cr.openjdk.java.net/~rkennke/8189373/webrev.00/ <http://cr.openjdk.java.net/%7Erkennke/8189373/webrev.00/>
>>>>
>>>> I'll do the same for SerialHeap once the above has been approved and pushed, otherwise it'll be a mess. ;-)
>>>>
>>>> Roman
>>> The SA strikes again! Yes, it looks like the same thing should be done for SerialHeap.
>>> I’m going to leave the review of 8189373 to others who have more clue about the SA.
>>>
>> Okidoki, so here comes the SerialGC with SA boilerplate:
>>
>> Differential:
>> http://cr.openjdk.java.net/~rkennke/8183542/webrev.02.diff/ <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.02.diff/>
>>
>> Full:
>> http://cr.openjdk.java.net/~rkennke/8183542/webrev.02/ <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.02/>
>>
>> This builds on top of the patch for https://bugs.openjdk.java.net/browse/JDK-8189373 which should land in the repo shortly, and implements the same thing for SerialHeap. It also passes the test that failed in the mentioned bug report (with -XX:+UseSerialGC).
>>
>> Can I get reviews (for the changed/added stuff) again?
>>
>> Thanks, Roman
> Looks good to me, though I don’t know much about the SA.
> It at least looks consistent with the changes for JDK-8189373.
>
More information about the serviceability-dev
mailing list