RFR (M): 8014659: NPG: performance counters for compressed klass space

harold seigel harold.seigel at oracle.com
Fri Aug 16 05:10:33 PDT 2013


Hi Erik,

Thanks for doing the work of merging our two changes.  I had one small 
comment.  In MetaspaceAux::reserved_in_bytes() could you change this

    size_t reserved = list == NULL ? 0 : list->virtual_space_total();
    return reserved;

to

    return list == NULL ? 0 : list->virtual_space_total();

Thanks, Harold


On 8/16/2013 6:22 AM, Erik Helin wrote:
> On 2013-08-15, Mikael Gerdin wrote:
>> Erik,
>>
>> On 08/14/2013 04:49 PM, Erik Helin wrote:
>>> Hi all,
>>>
>>> this change adds performance counters for compressed class space.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~ehelin/8014659/webrev.00/
>>>
>>> Changes to hotspot:
>>> The main changes are in metaspaceCounters.hpp and metaspaceCounters.cpp,
>>> where the class MetaspaceCounters has been split up into
>>> MetaspaceCounters and MetaspacePerfCounters. MetaspaceCounters now owns
>>> an instance of MetaspacePerfCounters. The class
>>> CompressedClassSpaceCounters has been added which also has its own
>>> instance of MetaspacePerfCounters. MetaspacePerfCounters initializes and
>>> updates the actual performance counters.
>> I'm a bit concerned about the exception handling in the perf
>> variable creation. But it appears to be similar to the other places
>> where perf variables are created.
>>
>> Creating a 0-reporting perf counter if -UseCompressedKlassPointers
>> seems consistent with the rest of the code.
>>
>> I'd say this is fine, but as Coleen mentioned you should verify this
>> with Harold's change for CDS+CompressedOops.
> Coleen, Mikael,
>
> I've rebased the change on top of hotspot-rt (which includes Harold's
> change). I had to resolve some merge conflicts in metaspace.cpp.
>
> Please see the latest (and hopefully final) webrev at:
> http://cr.openjdk.java.net/~ehelin/8014659/webrev.04/
>
> The incremental changes since the first webrev are listed below:
> - http://cr.openjdk.java.net/~ehelin/8014659/webrev.03-04/
> - http://cr.openjdk.java.net/~ehelin/8014659/webrev.02-03/
> - http://cr.openjdk.java.net/~ehelin/8014659/webrev.01-02/
> - http://cr.openjdk.java.net/~ehelin/8014659/webrev.00-01/
>
> I plan to push this to hotspot-rt as well, since the change now depends
> on Harold's change.
>
> Thanks,
> Erik
>
>>> The changes in metaspace.hpp/cpp were needed to get some additional data
>> >from the metaspace data structures. The method
>>> free_chunks_in_total(mdtype) was made public and the method
>>> free_bytes(mdtype) was added. Some common functionality was extracted
>>> into get_space_list(mdtype) which got rid of some duplicated code.
>>>
>>> The changes in:
>>> - g1MonitorinSupport.cpp
>>> - parallelScavengeHeap.cpp
>>> - genCollectedHeap.cpp
>>> - universe.cpp
>>> are only "one-liners" that either update or initialize the new performance
>>> counters.
>>>
>>> Changes to the testlibrary:
>>> - Added Asserts.java for writing asserts like "assertTrue",
>>>    "assertEquals", etc.
>>> - Added PerfCounter.java and PerfCounters.java to make it easy to
>>>    inspect performance counters for the currently running VM.
>>> - Added InputArguments.java so a test can check the arguments it got
>>>    passed.
>>> - Added InMemoryJavaCompiler.java for compiling a string into bytecode.
>>>    Useful for loading classes generated at runtime without using files.
>>> - Added ByteCodeLoader.java for defining a new class when you already
>>>    have the bytecode.
>> The test code looks fine.
>>
>> /Mikael
>>> Testing:
>>> - Added the new test TestMetaspacePerfCounters.java
>>> - JPRT
>>>
>>> Bug:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014659
>>>
>>> Thanks,
>>> Erik
>>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130816/b49390f8/attachment.html 


More information about the hotspot-runtime-dev mailing list