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