RFR (S): 8004172: Update jstat counter names to reflect metaspace changes

[Erik Helin]

Hi Stefan,

thanks for reviewing!

See new webrev for hotspot at:
http://cr.openjdk.java.net/~ehelin/8004172/hotspot/webrev.02/

On 01/30/2013 10:56 AM, Stefan Karlsson wrote:
> On 28 jan 2013, at 16:14, Erik Helin <erik.helin at oracle.com> wrote:
>> Webrev:
>> - hotspot: http://cr.openjdk.java.net/~ehelin/8004172/hotspot/webrev.01/
>
> Would you mind using two indentation levels here:
> +MetaspaceCounters::MetaspaceCounters() :
> +  _capacity(NULL),
> +  _used(NULL),
> +  _max_capacity(NULL) {

Fixed!

On 01/30/2013 10:56 AM, Stefan Karlsson wrote:
> I think it would be good to also extract this:
>    68     const char *counter_name = PerfDataManager::counter_name(ms, "minCapacity");
>    69     PerfDataManager::create_constant(SUN_RT, counter_name, PerfData::U_Bytes,
>
> into a create_ms_constant, just like you did with create_ms_variable.

Done!

On 01/30/2013 10:56 AM, Stefan Karlsson wrote:
> You should probably use CHECK instead of THREAD.
> +    _max_capacity = create_ms_variable(ms, "maxCapacity", max_capacity, THREAD);
> +    _capacity = create_ms_variable(ms, "capacity", curr_capacity, THREAD);
> +    _used = create_ms_variable(ms, "used", used, THREAD);

Agree, I've updated the code.

On 01/30/2013 10:56 AM, Stefan Karlsson wrote:
> I think it would be enough to assert that the variables are not NULL.
>   void MetaspaceCounters::update_capacity() {
>     assert(UsePerfData, "Should not be called unless being used");
>     size_t capacity_in_bytes = MetaspaceAux::capacity_in_bytes();
> +  if (_capacity != NULL) {

Agree, I've updated the code.

Thanks,
Erik

> thanks,
> StefanK
>
>> - jdk: http://cr.openjdk.java.net/~ehelin/8004172/jdk/webrev.01/
>>
>> What do you think?
>>
>> Thanks,
>> Erik
>>
>>> Jon
>>>
>>> On 1/24/2013 2:51 AM, Erik Helin wrote:
>>>> Hi all,
>>>>
>>>> here are the HotSpot changes for fixing JDK-8004172. This change uses
>>>> the new namespace "sun.gc.metaspace" for the metaspace counters and
>>>> also removes some code from metaspaceCounters.hpp/cpp that is not
>>>> needed any longer.
>>>>
>>>> Note that the tests will continue to fail until the JDK part of the
>>>> change finds it way into the hotspot-gc forest.
>>>>
>>>> The JDK part of the change is also out for review on
>>>> serviceability-dev at openjdk.java.net.
>>>>
>>>> Webrev:
>>>> HotSpot: http://cr.openjdk.java.net/~ehelin/8004172/hotspot/webrev.00/
>>>> JDK: http://cr.openjdk.java.net/~ehelin/8004172/jdk/webrev.00/
>>>>
>>>> Bug:
>>>> http://bugs.sun.com/view_bug.do?bug_id=8004172
>>>>
>>>> Testing:
>>>> Run the jstat jtreg tests locally on my machine on a repository where
>>>> I've applied both the JDK changes and the HotSpot changes.
>>>>
>>>> Thanks,
>>>> Erik
>>
>
>