<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix"><br>
Erik, Thank you for doing this.<br>
<br>
A small nit - can you put () around list == NULL?<br>
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span class="new" style="color: blue; font-weight: normal;">+ size_t reserved = list == NULL ? 0 : list->virtual_space_total();</span>
</pre>
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.<br>
<br>
Does MetaspaceCounters make sense if the CompressedClassSpace is
removed from the size calculations?<br>
<br>
Thanks,<br>
Coleen<br>
<br>
On 8/16/2013 6:22 AM, Erik Helin wrote:<br>
</div>
<blockquote cite="mid:20130816102208.GA2238@ehelin-thinkpad"
type="cite">
<pre wrap="">On 2013-08-15, Mikael Gerdin wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Erik,
On 08/14/2013 04:49 PM, Erik Helin wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi all,
this change adds performance counters for compressed class space.
Webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ehelin/8014659/webrev.00/">http://cr.openjdk.java.net/~ehelin/8014659/webrev.00/</a>
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.
</pre>
</blockquote>
<pre wrap="">
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.
</pre>
</blockquote>
<pre wrap="">
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:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ehelin/8014659/webrev.04/">http://cr.openjdk.java.net/~ehelin/8014659/webrev.04/</a>
The incremental changes since the first webrev are listed below:
- <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ehelin/8014659/webrev.03-04/">http://cr.openjdk.java.net/~ehelin/8014659/webrev.03-04/</a>
- <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ehelin/8014659/webrev.02-03/">http://cr.openjdk.java.net/~ehelin/8014659/webrev.02-03/</a>
- <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ehelin/8014659/webrev.01-02/">http://cr.openjdk.java.net/~ehelin/8014659/webrev.01-02/</a>
- <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ehelin/8014659/webrev.00-01/">http://cr.openjdk.java.net/~ehelin/8014659/webrev.00-01/</a>
I plan to push this to hotspot-rt as well, since the change now depends
on Harold's change.
Thanks,
Erik
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">The changes in metaspace.hpp/cpp were needed to get some additional data
</pre>
</blockquote>
<pre wrap="">>from the metaspace data structures. The method
</pre>
<blockquote type="cite">
<pre wrap="">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.
</pre>
</blockquote>
<pre wrap="">
The test code looks fine.
/Mikael
</pre>
<blockquote type="cite">
<pre wrap="">
Testing:
- Added the new test TestMetaspacePerfCounters.java
- JPRT
Bug:
<a class="moz-txt-link-freetext" href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014659">http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014659</a>
Thanks,
Erik
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
</blockquote>
<br>
</body>
</html>