Make reserved_size for compressed class space and metaspace respect the ergo-initialized CompressedClassSpaceSize flag value

[coleen.phillimore at oracle.com]

Hi this looks good and I will sponsor this with you as contributor using 
manc at google.com .

Have you signed the contributor agreement or are covered by one?

thanks,
Coleen

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