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

[Coleen Phillimore]

Erik,   Thank you for doing this.

A small nit - can you put () around list == NULL?

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

Also, I thought I read this intent of this change was to separate 
CompressedClassSpaceCounters from MetaspaceCounters, but 
MetaspaceCounters still call MetaspaceAux::free_bytes() and functions 
with combine the two.   I don't think we should combine them anymore.   
I'm working on decoupling these two with LowMemoryThreshold counters 
(which you have to see).   It's sort of a mess from the users 
perspective when these are combined.

Does MetaspaceCounters make sense if the CompressedClassSpace is removed 
from the size calculations?

Thanks,
Coleen

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: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130816/05cf6148/attachment.htm>