RFR: 8189733: Cleanup Full GC setup and tear down

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8189733: Cleanup Full GC setup and tear down

Stefan Johansson
Hi,

Please review this enhancement:
https://bugs.openjdk.java.net/browse/JDK-8189733

Webrev:
http://cr.openjdk.java.net/~sjohanss//8189733/00/index.html

Summary:
After the G1FullCollector has been introduced as part of JEP 307 it
makes sense to move more of the setup and tear down for the Full GC into
this class. The G1FullGCScope is also moved into the G1FullCollector
since it is now only used here.

Testing:
* Tier 1-2

Thanks,
Stefan


Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8189733: Cleanup Full GC setup and tear down

Thomas Schatzl
Hi,

On Wed, 2017-11-15 at 17:24 +0100, Stefan Johansson wrote:

> Hi,
>
> Please review this enhancement:
> https://bugs.openjdk.java.net/browse/JDK-8189733
>
> Webrev:
> http://cr.openjdk.java.net/~sjohanss//8189733/00/index.html
>
> Summary:
> After the G1FullCollector has been introduced as part of JEP 307 it 
> makes sense to move more of the setup and tear down for the Full GC
> into 
> this class. The G1FullGCScope is also moved into the G1FullCollector 
> since it is now only used here.

  looks good.

Thomas

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8189733: Cleanup Full GC setup and tear down

Stefan Johansson
Thanks Thomas for the review,

I got some comments from Erik and have updated the review accordingly.
Here are the new webrevs:
Full: http://cr.openjdk.java.net/~sjohanss/8189733/01/
Inc: http://cr.openjdk.java.net/~sjohanss/8189733/00-01/

This change removes the heap() function as it's only used by member
functions which can use _heap. Also changed the initialization list to
use the parameters rather than the members initialized.

Thanks,
Stefan

On 2017-11-16 09:31, Thomas Schatzl wrote:

> Hi,
>
> On Wed, 2017-11-15 at 17:24 +0100, Stefan Johansson wrote:
>> Hi,
>>
>> Please review this enhancement:
>> https://bugs.openjdk.java.net/browse/JDK-8189733
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sjohanss//8189733/00/index.html
>>
>> Summary:
>> After the G1FullCollector has been introduced as part of JEP 307 it
>> makes sense to move more of the setup and tear down for the Full GC
>> into
>> this class. The G1FullGCScope is also moved into the G1FullCollector
>> since it is now only used here.
>    looks good.
>
> Thomas
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8189733: Cleanup Full GC setup and tear down

Thomas Schatzl
Hi,

On Wed, 2017-11-22 at 16:39 +0100, Stefan Johansson wrote:

> Thanks Thomas for the review,
>
> I got some comments from Erik and have updated the review
> accordingly.
> Here are the new webrevs:
> Full: http://cr.openjdk.java.net/~sjohanss/8189733/01/
> Inc: http://cr.openjdk.java.net/~sjohanss/8189733/00-01/
>
> This change removes the heap() function as it's only used by member
> functions which can use _heap. Also changed the initialization list
> to use the parameters rather than the members initialized.
>

  still good.

Thomas

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8189733: Cleanup Full GC setup and tear down

Erik Helin-2
In reply to this post by Stefan Johansson
On 11/22/2017 04:39 PM, Stefan Johansson wrote:
> Thanks Thomas for the review,
>
> I got some comments from Erik and have updated the review accordingly.
> Here are the new webrevs:
> Full: http://cr.openjdk.java.net/~sjohanss/8189733/01/
> Inc: http://cr.openjdk.java.net/~sjohanss/8189733/00-01/

Looks good, Reviewed. Nice cleanup!

Thanks,
Erik

> This change removes the heap() function as it's only used by member
> functions which can use _heap. Also changed the initialization list to
> use the parameters rather than the members initialized.
>
> Thanks,
> Stefan
>
> On 2017-11-16 09:31, Thomas Schatzl wrote:
>> Hi,
>>
>> On Wed, 2017-11-15 at 17:24 +0100, Stefan Johansson wrote:
>>> Hi,
>>>
>>> Please review this enhancement:
>>> https://bugs.openjdk.java.net/browse/JDK-8189733
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sjohanss//8189733/00/index.html
>>>
>>> Summary:
>>> After the G1FullCollector has been introduced as part of JEP 307 it
>>> makes sense to move more of the setup and tear down for the Full GC
>>> into
>>> this class. The G1FullGCScope is also moved into the G1FullCollector
>>> since it is now only used here.
>>    looks good.
>>
>> Thomas
>>
>