<div dir="ltr">Hi Yasumasa,<div><br></div><div>Thanks for the review! Although I think moving or not moving "CompressedClassSpaceSize = align_size_down_bounded()" are equivalent because of logic of alignment and <span style="font-size:12.8px">FLAG_SET_ERGO</span>, I made the change you suggested anyway.</div><div>The new patch is attached and inlined below.<br></div><div><br></div><div><br><div><div><font face="monospace, monospace">--- old/src/hotspot/share/memory/metaspace.cpp<span style="white-space:pre">    </span>2017-12-08 18:42:48.960285998 -0800</font></div><div><font face="monospace, monospace">+++ new/src/hotspot/share/memory/metaspace.cpp<span style="white-space:pre">      </span>2017-12-08 18:42:48.600288972 -0800</font></div><div><font face="monospace, monospace">@@ -3322,7 +3322,6 @@</font></div><div><font face="monospace, monospace">   MaxMetaspaceExpansion = align_down_bounded(MaxMetaspaceExpansion, _commit_alignment);</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace">   CompressedClassSpaceSize = align_down_bounded(CompressedClassSpaceSize, _reserve_alignment);</font></div><div><font face="monospace, monospace">-  set_compressed_class_space_size(CompressedClassSpaceSize);</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace">   // Initial virtual space size will be calculated at global_initialize()</font></div><div><font face="monospace, monospace">   size_t min_metaspace_sz =</font></div><div><font face="monospace, monospace">@@ -3341,6 +3340,7 @@</font></div><div><font face="monospace, monospace">                   min_metaspace_sz);</font></div><div><font face="monospace, monospace">   }</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace">+  set_compressed_class_space_size(CompressedClassSpaceSize);</font></div><div><font face="monospace, monospace"> }</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace"> void Metaspace::global_initialize() {</font></div></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"><br></font></div><div class="gmail_extra">Thanks,<br clear="all"><div><div class="gmail_signature"><div dir="ltr">Man</div></div></div>
<br><div class="gmail_quote">On Thu, Dec 7, 2017 at 9:44 PM, Yasumasa Suenaga <span dir="ltr"><<a href="mailto:yasuenag@gmail.com" target="_blank">yasuenag@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Man,<br>
<br>
CompressedClassSpaceSize might be modified by FLAG_SET_ERGO.<br>
So I think you need to move set_compressed_class_space_<wbr>size() only.<br>
<br>
<br>
Thanks,<br>
<br>
Yasumasa<br>
<div class="gmail-HOEnZb"><div class="gmail-h5"><br>
<br>
<br>
2017-12-08 3:42 GMT+09:00 Man Cao <<a href="mailto:manc@google.com">manc@google.com</a>>:<br>
> Hello,<br>
><br>
> This is a friendly ping. Could anyone review or sponsor this change? It's<br>
> just a two-liner change.<br>
><br>
> -Man<br>
><br>
> On Thu, Nov 30, 2017 at 2:03 PM, Man Cao <<a href="mailto:manc@google.com">manc@google.com</a>> wrote:<br>
>><br>
>> I realized that the email attachment is probably dropped by the mailing<br>
>> list, so below is the inlined patch.<br>
>><br>
>> --- old/src/hotspot/share/memory/<wbr>metaspace.cpp 2017-11-29<br>
>> 14:56:59.017118444 -0800<br>
>> +++ new/src/hotspot/share/memory/<wbr>metaspace.cpp 2017-11-29<br>
>> 14:56:58.657121375 -0800<br>
>> @@ -3321,9 +3321,6 @@<br>
>>    MinMetaspaceExpansion = align_down_bounded(<wbr>MinMetaspaceExpansion,<br>
>> _commit_alignment);<br>
>>    MaxMetaspaceExpansion = align_down_bounded(<wbr>MaxMetaspaceExpansion,<br>
>> _commit_alignment);<br>
>><br>
>> -  CompressedClassSpaceSize = align_down_bounded(<wbr>CompressedClassSpaceSize,<br>
>> _reserve_alignment);<br>
>> -  set_compressed_class_space_<wbr>size(CompressedClassSpaceSize)<wbr>;<br>
>> -<br>
>>    // Initial virtual space size will be calculated at global_initialize()<br>
>>    size_t min_metaspace_sz =<br>
>>        VIRTUALSPACEMULTIPLIER * InitialBootClassLoaderMetaspac<wbr>eSize;<br>
>> @@ -3341,6 +3338,8 @@<br>
>>                    min_metaspace_sz);<br>
>>    }<br>
>><br>
>> +  CompressedClassSpaceSize = align_down_bounded(<wbr>CompressedClassSpaceSize,<br>
>> _reserve_alignment);<br>
>> +  set_compressed_class_space_<wbr>size(CompressedClassSpaceSize)<wbr>;<br>
>>  }<br>
>><br>
>>  void Metaspace::global_initialize() {<br>
>><br>
>> Best,<br>
>> Man<br>
>><br>
>> On Wed, Nov 29, 2017 at 3:21 PM, Man Cao <<a href="mailto:manc@google.com">manc@google.com</a>> wrote:<br>
>>><br>
>>> Hello,<br>
>>><br>
>>> This patch is a follow-up fix for<br>
>>> <a href="https://bugs.openjdk.java.net/browse/JDK-8087291" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/<wbr>browse/JDK-8087291</a><br>
>>><br>
>>> This patch moves the call to set_compressed_class_space_<wbr>size() after the<br>
>>> flag value for CompressedClassSpaceSize is ergo-initialized, fixing the<br>
>>> issue that the reserved size for compressed class space and metaspace is<br>
>>> excessively large when MaxMetaspaceSize is set to a small value. More<br>
>>> discussion about it is available here:<br>
>>><br>
>>> <a href="http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2017-November/025200.html" rel="noreferrer" target="_blank">http://mail.openjdk.java.net/<wbr>pipermail/hotspot-runtime-dev/<wbr>2017-November/025200.html</a><br>
>>><br>
>>> This code patch is attached. Could anyone review and/or sponsor this<br>
>>> patch?<br>
>>><br>
>>> Best,<br>
>>> Man<br>
>><br>
>><br>
><br>
</div></div></blockquote></div><br></div></div></div>