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

[Yasumasa Suenaga]

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
>      >>
>      >>
>      >
> 
>