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

[Man Cao]

Hi Coleen,

Thanks for sponsoring this patch! Yes, I'm covered by the agreement signed
by Google Inc. [1]
Could this patch be a part of
https://bugs.openjdk.java.net/browse/JDK-8087291, as this is a simple
follow-up fix?
If not, could you or someone else create a bug, as I don't have an account
to create one?

[1] http://www.oracle.com/technetwork/community/oca-486395.html#g

-Man

On Mon, Dec 11, 2017 at 2:27 PM, <coleen.phillimore at oracle.com> wrote:

> 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(MinMetaspac
>>> eExpansion,
>>>      >> _commit_alignment);
>>>      >>    MaxMetaspaceExpansion = align_down_bounded(MaxMetaspac
>>> eExpansion,
>>>      >> _commit_alignment);
>>>      >>
>>>      >> -  CompressedClassSpaceSize = align_down_bounded(CompressedC
>>> lassSpaceSize,
>>>      >> _reserve_alignment);
>>>      >> - set_compressed_class_space_size(CompressedClassSpaceSize);
>>>      >> -
>>>      >>    // Initial virtual space size will be calculated at
>>> global_initialize()
>>>      >>    size_t min_metaspace_sz =
>>>      >>        VIRTUALSPACEMULTIPLIER * InitialBootClassLoaderMetaspac
>>> eSize;
>>>      >> @@ -3341,6 +3338,8 @@
>>>      >>                    min_metaspace_sz);
>>>      >>    }
>>>      >>
>>>      >> +  CompressedClassSpaceSize = align_down_bounded(CompressedC
>>> lassSpaceSize,
>>>      >> _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/2
>>> 017-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
>>>      >>
>>>      >>
>>>      >
>>>
>>>
>>>
>