Make reserved_size for compressed class space and metaspace respect the ergo-initialized CompressedClassSpaceSize flag value
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Tue Dec 12 16:29:16 UTC 2017
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
> <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
> <mailto:coleen.phillimore at oracle.com>> wrote:
>
> Hi this looks good and I will sponsor this with you as contributor
> using manc at google.com <mailto: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>
> <mailto: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> <mailto: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>
> <mailto: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>
> <mailto: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>
> <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>
> <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
> >>
> >>
> >
>
>
>
>
More information about the hotspot-runtime-dev
mailing list