<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">On 01/24/2013 05:38 AM, Jon Masamitsu
wrote:<br>
</div>
<blockquote cite="mid:5100BABB.4040004@oracle.com" type="cite">Coleen,
<br>
<br>
Thanks for the review.
<br>
<br>
I delete the print at 1013 (instead of moving it) and reverted
get_new_chunk(). I left in the
<br>
print at 1014.
<br>
<br>
I have 2 webrevs for these changes now.</blockquote>
<br>
Thanks for splitting this into two changes.<br>
<br>
<blockquote cite="mid:5100BABB.4040004@oracle.com" type="cite">
Your suggested changes are in
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/8006815/webrev.00/">http://cr.openjdk.java.net/~jmasa/8006815/webrev.00/</a>
<br>
</blockquote>
<br>
I don't know if this is a reasonable change or not.<br>
<br>
Why are you checking if we should expand, before trying to allocate
in the current virtual space?<br>
<pre><span class="new"> 982 // The next attempts at allocating a chunk will expand the</span>
<span class="new"> 983 // Metaspace capacity. Check first if there should be an expansion.</span>
<span class="new"> 984 if (!MetaspaceGC::should_expand(this, word_size, grow_chunks_by_words)) {</span>
<span class="new"> 985 return next;</span>
<span class="new"> 986 }</span>
<span class="new"> 987 </span>
988 // Allocate a chunk out of the current virtual space.
989 if (next == NULL) {
990 next = current_virtual_space()->get_chunk_vs(grow_chunks_by_words);
991 }
</pre>
Shouldn't this line be checking "less than or equal":<br>
<pre><b><font color="blue">!</font><font color="black"> if (</font><u><font color="blue">(</font></u><font color="black">vsl->capacity_words_sum(</font><u><font color="blue">) + expansion_word_size </font></u><font color="black">) < metaspace_size_words ||</font></b>
<font color="black"> capacity_until_GC() == 0) {</font>
<font color="black"> set_capacity_until_GC(metaspace_size_words);</font>
<font color="black"> return true;</font>
<font color="black"> }</font>
</pre>
<blockquote cite="mid:5100BABB.4040004@oracle.com" type="cite">Webrev
with the Min/MaxMetaspaceFreeRatio changes is
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/8005452/webrev.02/">http://cr.openjdk.java.net/~jmasa/8005452/webrev.02/</a>
<br>
</blockquote>
<br>
This looks good.<br>
<br>
Though, I think you need to update the descriptions of the new
flags:<br>
+ product(uintx, MinMetaspaceFreeRatio,
10, \<br>
+ "Min percentage of heap free after GC to avoid
expansion") \<br>
+
\<br>
+ product(uintx, MaxMetaspaceFreeRatio,
20, \<br>
+ "Max percentage of heap free after GC to avoid
shrinking") \<br>
<br>
thanks,<br>
StefanK<br>
<br>
<blockquote cite="mid:5100BABB.4040004@oracle.com" type="cite">
<br>
Jon
<br>
<br>
On 1/23/2013 9:01 AM, Coleen Phillimore wrote:
<br>
<blockquote type="cite">
<br>
It looks okay except I think passing SpaceManager* to
get_new_chunk() is really gross just to do a print out. I'd
rather see the printing at line 1014 moved to 2170 whether you
get a new virtual space or not. There's already a ton of output
from TraceMetadataChunkAllocation && Verbose, but you
can leave the printing at 1013 so you know which one created a
new virtual space.
<br>
<br>
Coleen
<br>
<br>
<br>
On 01/14/2013 10:30 AM, Jon Masamitsu wrote:
<br>
<blockquote type="cite">8005452: Create new flags for Metaspace
resizing policy
<br>
<br>
Previously the calculation of the metadata capacity at which
<br>
to do a GC (high water mark, HWM) to recover
<br>
unloaded classes used the MinHeapFreeRatio
<br>
and MaxHeapFreeRatio to decide on the next HWM. That
<br>
generally left an excessive amount of unused capacity for
<br>
metadata. This change adds specific flags for metadata
<br>
capacity with defaults more conservative in terms of
<br>
unused capacity.
<br>
<br>
Added an additional check for doing a GC before expanding
<br>
the metadata capacity. Required adding a new parameter to
<br>
get_new_chunk().
<br>
<br>
Added some additional diagnostic prints.
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/8005452/webrev.00/">http://cr.openjdk.java.net/~jmasa/8005452/webrev.00/</a>
<br>
<br>
Thanks.
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</body>
</html>