RFR: 8191861: Move and refactor hSpaceCounters

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

RFR: 8191861: Move and refactor hSpaceCounters

Stefan Karlsson
Hi all,

Please review this patch to move and refactor HSpaceCounters.

http://cr.openjdk.java.net/~stefank/8191861/webrev.01
https://bugs.openjdk.java.net/browse/JDK-8191861

The patch moves hSpaceCounters.hpp/cpp out from gc/g1 into gc/shared, so
that it can be reused by other GCs.

The patch also removes the dependency between HSpaceCounters and
GenerationCounters. The passed in GenerationCounters object was only
used to provide the name_space of the perf counter. This patch changes
G1 to explicitly pass in the name_space string instead of the
GenerationCounters.

This patch builds upon these two patches:
 
http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029267.html
 
http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029268.html

Thanks,
StefanK
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191861: Move and refactor hSpaceCounters

Per Liden
Looks good!

/Per

On 2017-11-24 14:38, Stefan Karlsson wrote:

> Hi all,
>
> Please review this patch to move and refactor HSpaceCounters.
>
> http://cr.openjdk.java.net/~stefank/8191861/webrev.01
> https://bugs.openjdk.java.net/browse/JDK-8191861
>
> The patch moves hSpaceCounters.hpp/cpp out from gc/g1 into gc/shared, so
> that it can be reused by other GCs.
>
> The patch also removes the dependency between HSpaceCounters and
> GenerationCounters. The passed in GenerationCounters object was only
> used to provide the name_space of the perf counter. This patch changes
> G1 to explicitly pass in the name_space string instead of the
> GenerationCounters.
>
> This patch builds upon these two patches:
>
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029267.html
>
>
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029268.html
>
>
> Thanks,
> StefanK
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191861: Move and refactor hSpaceCounters

Erik Helin-2
In reply to this post by Stefan Karlsson
On 11/24/2017 02:38 PM, Stefan Karlsson wrote:
> Hi all,
>
> Please review this patch to move and refactor HSpaceCounters.
>
> http://cr.openjdk.java.net/~stefank/8191861/webrev.01
> https://bugs.openjdk.java.net/browse/JDK-8191861
>
> The patch moves hSpaceCounters.hpp/cpp out from gc/g1 into gc/shared, so
> that it can be reused by other GCs.

Looks good, but could you please add a local variable for the expression

_young_collection_counters->name_space()

that is repeated three times now in g1MonitoringSupport.cpp. I don't
need a re-review for this small additional change.

Thanks,
Erik

> The patch also removes the dependency between HSpaceCounters and
> GenerationCounters. The passed in GenerationCounters object was only
> used to provide the name_space of the perf counter. This patch changes
> G1 to explicitly pass in the name_space string instead of the
> GenerationCounters.
>
> This patch builds upon these two patches:
>
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029267.html 
>
>
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029268.html 
>
>
> Thanks,
> StefanK
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191861: Move and refactor hSpaceCounters

Roman Kennke-6
In reply to this post by Stefan Karlsson
Looks good to me too.

This is welcome because it's one oft the parts that Shenandoah uses from G1.

Thanks, Roman


Am 24. November 2017 14:38:38 MEZ schrieb Stefan Karlsson <[hidden email]>:
Hi all,

Please review this patch to move and refactor HSpaceCounters.

http://cr.openjdk.java.net/~stefank/8191861/webrev.01
https://bugs.openjdk.java.net/browse/JDK-8191861

The patch moves hSpaceCounters.hpp/cpp out from gc/g1 into gc/shared, so
that it can be reused by other GCs.

The patch also removes the dependency between HSpaceCounters and
GenerationCounters. The passed in GenerationCounters object was only
used to provide the name_space of the perf counter. This patch changes
G1 to explicitly pass in the name_space string instead of the
GenerationCounters.

This patch builds upon these two patches:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029267.html

http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029268.html

Thanks,
StefanK

--
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191861: Move and refactor hSpaceCounters

Stefan Karlsson
In reply to this post by Per Liden
Thanks, Per!

StefanK

On 2017-11-24 15:05, Per Liden wrote:

> Looks good!
>
> /Per
>
> On 2017-11-24 14:38, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to move and refactor HSpaceCounters.
>>
>> http://cr.openjdk.java.net/~stefank/8191861/webrev.01
>> https://bugs.openjdk.java.net/browse/JDK-8191861
>>
>> The patch moves hSpaceCounters.hpp/cpp out from gc/g1 into gc/shared, so
>> that it can be reused by other GCs.
>>
>> The patch also removes the dependency between HSpaceCounters and
>> GenerationCounters. The passed in GenerationCounters object was only
>> used to provide the name_space of the perf counter. This patch changes
>> G1 to explicitly pass in the name_space string instead of the
>> GenerationCounters.
>>
>> This patch builds upon these two patches:
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029267.html 
>>
>>
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029268.html 
>>
>>
>>
>> Thanks,
>> StefanK
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191861: Move and refactor hSpaceCounters

Stefan Karlsson
In reply to this post by Erik Helin-2
On 2017-11-24 15:11, Erik Helin wrote:

> On 11/24/2017 02:38 PM, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to move and refactor HSpaceCounters.
>>
>> http://cr.openjdk.java.net/~stefank/8191861/webrev.01
>> https://bugs.openjdk.java.net/browse/JDK-8191861
>>
>> The patch moves hSpaceCounters.hpp/cpp out from gc/g1 into gc/shared,
>> so that it can be reused by other GCs.
>
> Looks good, but could you please add a local variable for the expression
>
> _young_collection_counters->name_space()
>
> that is repeated three times now in g1MonitoringSupport.cpp. I don't
> need a re-review for this small additional change.

Done.

Thanks for the review!

StefanK

>
> Thanks,
> Erik
>
>> The patch also removes the dependency between HSpaceCounters and
>> GenerationCounters. The passed in GenerationCounters object was only
>> used to provide the name_space of the perf counter. This patch changes
>> G1 to explicitly pass in the name_space string instead of the
>> GenerationCounters.
>>
>> This patch builds upon these two patches:
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029267.html 
>>
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029268.html 
>>
>>
>> Thanks,
>> StefanK
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191861: Move and refactor hSpaceCounters

Stefan Karlsson
In reply to this post by Roman Kennke-6
Thanks, Roman!

StefanK

On 2017-11-24 15:11, Roman Kennke wrote:

> Looks good to me too.
>
> This is welcome because it's one oft the parts that Shenandoah uses from G1.
>
> Thanks, Roman
>
>
> Am 24. November 2017 14:38:38 MEZ schrieb Stefan Karlsson
> <[hidden email]>:
>
>     Hi all,
>
>     Please review this patch to move and refactor HSpaceCounters.
>
>     http://cr.openjdk.java.net/~stefank/8191861/webrev.01
>     https://bugs.openjdk.java.net/browse/JDK-8191861
>
>     The patch moves hSpaceCounters.hpp/cpp out from gc/g1 into gc/shared, so
>     that it can be reused by other GCs.
>
>     The patch also removes the dependency between HSpaceCounters and
>     GenerationCounters. The passed in GenerationCounters object was only
>     used to provide the name_space of the perf counter. This patch changes
>     G1 to explicitly pass in the name_space string instead of the
>     GenerationCounters.
>
>     This patch builds upon these two patches:
>      
>     http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029267.html
>      
>     http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-November/029268.html
>
>     Thanks,
>     StefanK
>
>
> --
> Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.