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

Man Cao manc at google.com
Tue Dec 12 19:04:48 UTC 2017


Great! Thanks again!

-Man

On Tue, Dec 12, 2017 at 8:29 AM, <coleen.phillimore at oracle.com> wrote:

> Man, It has to be a new bug.  I filed bug:
> https://bugs.openjdk.java.net/browse/JDK-8193386
>
> and will sponsor this.
> thank you,
> Coleen
>
>
> On 12/11/17 7:38 PM, Man Cao wrote:
>
> 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
>>>>      >>
>>>>      >>
>>>>      >
>>>>
>>>>
>>>>
>>
>
>


More information about the hotspot-runtime-dev mailing list