Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
Hi,

(CC'ed hotspot-runtime-dev)

> I think the reason is that this bug is a P5. The compressed class space belongs to the runtime code, so you might get more traction for this on the hotspot-runtime-dev list.

I will send review request against jdk10/master or jdk10/hs after repos are opened.


Thanks,

Yasumasa


On 2017/09/20 20:53, Stefan Karlsson wrote:

> Hi Man,
>
> On 2017-09-13 20:55, Man Cao wrote:
>> Hi Yasumasa, Stefan,
>>
>> Do you have any thoughts on why this patch has been pending for 2+ years? This patch could really save us from some annoying issues since we are automatically monitoring hsperfdata counters.
>
> I think the reason is that this bug is a P5. The compressed class space belongs to the runtime code, so you might get more traction for this on the hotspot-runtime-dev list.
>
> StefanK
>
>>
>> -Man
>>
>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email] <mailto:[hidden email]>> wrote:
>>
>>     Hi all,
>>
>>     I wonder if there is any recent update on the patch for JDK-8087291.
>>     Is it possible to push this patch into JDK9? Except for its low
>>     priority (P5),
>>     is there any complication that prevents this patch getting approved
>>     (for example, some JVM logic requires CompressedClassSpaceSize to be
>>     1GB by default)?
>>
>>     I work in the Java Platform Team at Google. We have encountered
>>     annoying issues that the hsperfdata counter
>>     "sun_gc_metaspace_maxCapacity" reporting
>>     a too large value (about 1GB) even if user sets
>>     -XX:MaxMetaspaceSize=100m, as well as GC log shows the confusing 1GB
>>     memory reserved by metaspace,
>>     regardless of MaxMetaspaceSize value. The root cause for these
>>     issues is that CompressedClassSpaceSize is not automatically capped
>>     by MaxMetaspaceSize
>>     during VM initialization, and this patch seems fix the root cause.
>>     (I'm aware that even after this patch, the reserved size could still
>>     be up to 2*MaxMetaspaceSize,
>>     but it is better than the current situation.)
>>
>>     Thanks,
>>     Man
>>
>>     On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>
>>         Thank you for your comment!
>>          > Try running a debug JVM with your patch with this command line.
>>          >
>>          > java -XX:MaxMetaspaceSize=4195328 -version
>>         Sorry, I've fixed it and uploaded webrev:
>>         http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>         <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>         It works on fastdebug VM.
>>         Please review again.
>>
>>         Thanks,
>>         Yasumasa
>>
>>         On 2015/06/18 10:45, Jon Masamitsu wrote:
>>          > Yasumasa,
>>          >
>>          > Try running a debug JVM with your patch with this command line.
>>          >
>>          > java -XX:MaxMetaspaceSize=4195328 -version
>>          >
>>          > On a linux system I get this when I build with your patch.
>>          >
>>          >> java -XX:MaxMetaspaceSize=4195328 -version
>>          >> # To suppress the following error report, specify this argument
>>          >> # after -XX: or in .hotspotrc:
>>         SuppressErrorAt=/metaspace.cpp:2324
>>          >> #
>>          >> # A fatal error has been detected by the Java Runtime
>>         Environment:
>>          >> #
>>          >> #  Internal Error
>>          >>
>>         (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>          >> pid=19099, tid=0x00007ff4b9b92700
>>          >> #  assert(size > MediumChunk || size > ClassMediumChunk)
>>         failed: Not a
>>          >> humongous chunk
>>          >> #
>>          >
>>          >
>>          > Jon
>>          >
>>          >
>>          > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>          >> I want to continue to discuss about CompressedClassSpace and
>>         MaxMetaspace in this (RFR) thread.
>>          >>
>>          >>
>>         http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>         <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>          >>>> Should I resize CompressedClassSpaceSize than to show
>>         error message?
>>          >>> If you add slightly better heuristics for the setup of the
>>         CompressedClassSpaceSize flag, for example lowering the
>>         CompressedClassSpaceSize when MaxMetaspaceSize is set, then it
>>         might be less likely that you'll hit the OutOfMemoryError when
>>         the system is set up with strict overcommit settings.
>>          >>
>>          >> I've uploaded new webrev:
>>          >> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>         <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>          >>
>>          >> This patch checkes MaxMetaspaceSize,
>>         CompressedClassSpaceSize, and
>>          >> InitialBootClassLoaderMetaspaceSize.
>>          >>
>>          >> I add to check CompressedClassSpaceSize in
>>         Arguments::set_use_compressed_klass_ptrs().
>>          >> If InitialBootClassLoaderMetaspaceSize is greater than
>>         MaxMetaspaceSize,
>>          >> VM will fail with error message.
>>          >>
>>          >> InitialBootClassLoaderMetaspaceSize will be set to
>>         MaxMetaspaceSize
>>          >> when UseCompressedClassPointers is not set in
>>         Metaspace::ergo_initialize().
>>          >>
>>          >>
>>          >> Thanks,
>>          >>
>>          >> Yasumasa
>>          >>
>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Man Cao
Thank Yasumasa and Stefan for the responses.

Good to know that the patch is not blocked due to breaking some internal
invariants/assumptions, but just due to its P5 status.
Is it possible to push it to P4?

-Man

On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga <[hidden email]>
wrote:

> Hi,
>
> (CC'ed hotspot-runtime-dev)
>
> I think the reason is that this bug is a P5. The compressed class space
>> belongs to the runtime code, so you might get more traction for this on the
>> hotspot-runtime-dev list.
>>
>
> I will send review request against jdk10/master or jdk10/hs after repos
> are opened.
>
>
> Thanks,
>
> Yasumasa
>
>
>
> On 2017/09/20 20:53, Stefan Karlsson wrote:
>
>> Hi Man,
>>
>> On 2017-09-13 20:55, Man Cao wrote:
>>
>>> Hi Yasumasa, Stefan,
>>>
>>> Do you have any thoughts on why this patch has been pending for 2+
>>> years? This patch could really save us from some annoying issues since we
>>> are automatically monitoring hsperfdata counters.
>>>
>>
>> I think the reason is that this bug is a P5. The compressed class space
>> belongs to the runtime code, so you might get more traction for this on the
>> hotspot-runtime-dev list.
>>
>> StefanK
>>
>>
>>> -Man
>>>
>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email] <mailto:
>>> [hidden email]>> wrote:
>>>
>>>     Hi all,
>>>
>>>     I wonder if there is any recent update on the patch for JDK-8087291.
>>>     Is it possible to push this patch into JDK9? Except for its low
>>>     priority (P5),
>>>     is there any complication that prevents this patch getting approved
>>>     (for example, some JVM logic requires CompressedClassSpaceSize to be
>>>     1GB by default)?
>>>
>>>     I work in the Java Platform Team at Google. We have encountered
>>>     annoying issues that the hsperfdata counter
>>>     "sun_gc_metaspace_maxCapacity" reporting
>>>     a too large value (about 1GB) even if user sets
>>>     -XX:MaxMetaspaceSize=100m, as well as GC log shows the confusing 1GB
>>>     memory reserved by metaspace,
>>>     regardless of MaxMetaspaceSize value. The root cause for these
>>>     issues is that CompressedClassSpaceSize is not automatically capped
>>>     by MaxMetaspaceSize
>>>     during VM initialization, and this patch seems fix the root cause.
>>>     (I'm aware that even after this patch, the reserved size could still
>>>     be up to 2*MaxMetaspaceSize,
>>>     but it is better than the current situation.)
>>>
>>>     Thanks,
>>>     Man
>>>
>>>     On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>
>>>         Thank you for your comment!
>>>          > Try running a debug JVM with your patch with this command
>>> line.
>>>          >
>>>          > java -XX:MaxMetaspaceSize=4195328 -version
>>>         Sorry, I've fixed it and uploaded webrev:
>>>         http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>         <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>         It works on fastdebug VM.
>>>         Please review again.
>>>
>>>         Thanks,
>>>         Yasumasa
>>>
>>>         On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>          > Yasumasa,
>>>          >
>>>          > Try running a debug JVM with your patch with this command
>>> line.
>>>          >
>>>          > java -XX:MaxMetaspaceSize=4195328 -version
>>>          >
>>>          > On a linux system I get this when I build with your patch.
>>>          >
>>>          >> java -XX:MaxMetaspaceSize=4195328 -version
>>>          >> # To suppress the following error report, specify this
>>> argument
>>>          >> # after -XX: or in .hotspotrc:
>>>         SuppressErrorAt=/metaspace.cpp:2324
>>>          >> #
>>>          >> # A fatal error has been detected by the Java Runtime
>>>         Environment:
>>>          >> #
>>>          >> #  Internal Error
>>>          >>
>>>         (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/
>>> metaspace.cpp:2324),
>>>          >> pid=19099, tid=0x00007ff4b9b92700
>>>          >> #  assert(size > MediumChunk || size > ClassMediumChunk)
>>>         failed: Not a
>>>          >> humongous chunk
>>>          >> #
>>>          >
>>>          >
>>>          > Jon
>>>          >
>>>          >
>>>          > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>          >> I want to continue to discuss about CompressedClassSpace and
>>>         MaxMetaspace in this (RFR) thread.
>>>          >>
>>>          >>
>>>         http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-J
>>> une/013873.html
>>>         <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-
>>> June/013873.html>
>>>          >>>> Should I resize CompressedClassSpaceSize than to show
>>>         error message?
>>>          >>> If you add slightly better heuristics for the setup of the
>>>         CompressedClassSpaceSize flag, for example lowering the
>>>         CompressedClassSpaceSize when MaxMetaspaceSize is set, then it
>>>         might be less likely that you'll hit the OutOfMemoryError when
>>>         the system is set up with strict overcommit settings.
>>>          >>
>>>          >> I've uploaded new webrev:
>>>          >> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>         <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>          >>
>>>          >> This patch checkes MaxMetaspaceSize,
>>>         CompressedClassSpaceSize, and
>>>          >> InitialBootClassLoaderMetaspaceSize.
>>>          >>
>>>          >> I add to check CompressedClassSpaceSize in
>>>         Arguments::set_use_compressed_klass_ptrs().
>>>          >> If InitialBootClassLoaderMetaspaceSize is greater than
>>>         MaxMetaspaceSize,
>>>          >> VM will fail with error message.
>>>          >>
>>>          >> InitialBootClassLoaderMetaspaceSize will be set to
>>>         MaxMetaspaceSize
>>>          >> when UseCompressedClassPointers is not set in
>>>         Metaspace::ergo_initialize().
>>>          >>
>>>          >>
>>>          >> Thanks,
>>>          >>
>>>          >> Yasumasa
>>>          >>
>>>
>>>
>>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
Hi all,

I uploaded webrev for this issue against jdk10/hs.
Could you review it?

  http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/


I cannot access JPRT. So I need a sponsor.


Thanks,

Yasumasa



2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:

> Thank Yasumasa and Stefan for the responses.
>
> Good to know that the patch is not blocked due to breaking some internal
> invariants/assumptions, but just due to its P5 status.
> Is it possible to push it to P4?
>
> -Man
>
> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga <[hidden email]>
> wrote:
>>
>> Hi,
>>
>> (CC'ed hotspot-runtime-dev)
>>
>>> I think the reason is that this bug is a P5. The compressed class space
>>> belongs to the runtime code, so you might get more traction for this on the
>>> hotspot-runtime-dev list.
>>
>>
>> I will send review request against jdk10/master or jdk10/hs after repos
>> are opened.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>
>>> Hi Man,
>>>
>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>
>>>> Hi Yasumasa, Stefan,
>>>>
>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>> years? This patch could really save us from some annoying issues since we
>>>> are automatically monitoring hsperfdata counters.
>>>
>>>
>>> I think the reason is that this bug is a P5. The compressed class space
>>> belongs to the runtime code, so you might get more traction for this on the
>>> hotspot-runtime-dev list.
>>>
>>> StefanK
>>>
>>>>
>>>> -Man
>>>>
>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>> <mailto:[hidden email]>> wrote:
>>>>
>>>>     Hi all,
>>>>
>>>>     I wonder if there is any recent update on the patch for JDK-8087291.
>>>>     Is it possible to push this patch into JDK9? Except for its low
>>>>     priority (P5),
>>>>     is there any complication that prevents this patch getting approved
>>>>     (for example, some JVM logic requires CompressedClassSpaceSize to be
>>>>     1GB by default)?
>>>>
>>>>     I work in the Java Platform Team at Google. We have encountered
>>>>     annoying issues that the hsperfdata counter
>>>>     "sun_gc_metaspace_maxCapacity" reporting
>>>>     a too large value (about 1GB) even if user sets
>>>>     -XX:MaxMetaspaceSize=100m, as well as GC log shows the confusing 1GB
>>>>     memory reserved by metaspace,
>>>>     regardless of MaxMetaspaceSize value. The root cause for these
>>>>     issues is that CompressedClassSpaceSize is not automatically capped
>>>>     by MaxMetaspaceSize
>>>>     during VM initialization, and this patch seems fix the root cause.
>>>>     (I'm aware that even after this patch, the reserved size could still
>>>>     be up to 2*MaxMetaspaceSize,
>>>>     but it is better than the current situation.)
>>>>
>>>>     Thanks,
>>>>     Man
>>>>
>>>>     On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>
>>>>         Thank you for your comment!
>>>>          > Try running a debug JVM with your patch with this command
>>>> line.
>>>>          >
>>>>          > java -XX:MaxMetaspaceSize=4195328 -version
>>>>         Sorry, I've fixed it and uploaded webrev:
>>>>         http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>         <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>         It works on fastdebug VM.
>>>>         Please review again.
>>>>
>>>>         Thanks,
>>>>         Yasumasa
>>>>
>>>>         On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>          > Yasumasa,
>>>>          >
>>>>          > Try running a debug JVM with your patch with this command
>>>> line.
>>>>          >
>>>>          > java -XX:MaxMetaspaceSize=4195328 -version
>>>>          >
>>>>          > On a linux system I get this when I build with your patch.
>>>>          >
>>>>          >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>          >> # To suppress the following error report, specify this
>>>> argument
>>>>          >> # after -XX: or in .hotspotrc:
>>>>         SuppressErrorAt=/metaspace.cpp:2324
>>>>          >> #
>>>>          >> # A fatal error has been detected by the Java Runtime
>>>>         Environment:
>>>>          >> #
>>>>          >> #  Internal Error
>>>>          >>
>>>>
>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>          >> pid=19099, tid=0x00007ff4b9b92700
>>>>          >> #  assert(size > MediumChunk || size > ClassMediumChunk)
>>>>         failed: Not a
>>>>          >> humongous chunk
>>>>          >> #
>>>>          >
>>>>          >
>>>>          > Jon
>>>>          >
>>>>          >
>>>>          > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>          >> I want to continue to discuss about CompressedClassSpace and
>>>>         MaxMetaspace in this (RFR) thread.
>>>>          >>
>>>>          >>
>>>>
>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>
>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>          >>>> Should I resize CompressedClassSpaceSize than to show
>>>>         error message?
>>>>          >>> If you add slightly better heuristics for the setup of the
>>>>         CompressedClassSpaceSize flag, for example lowering the
>>>>         CompressedClassSpaceSize when MaxMetaspaceSize is set, then it
>>>>         might be less likely that you'll hit the OutOfMemoryError when
>>>>         the system is set up with strict overcommit settings.
>>>>          >>
>>>>          >> I've uploaded new webrev:
>>>>          >> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>         <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>          >>
>>>>          >> This patch checkes MaxMetaspaceSize,
>>>>         CompressedClassSpaceSize, and
>>>>          >> InitialBootClassLoaderMetaspaceSize.
>>>>          >>
>>>>          >> I add to check CompressedClassSpaceSize in
>>>>         Arguments::set_use_compressed_klass_ptrs().
>>>>          >> If InitialBootClassLoaderMetaspaceSize is greater than
>>>>         MaxMetaspaceSize,
>>>>          >> VM will fail with error message.
>>>>          >>
>>>>          >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>         MaxMetaspaceSize
>>>>          >> when UseCompressedClassPointers is not set in
>>>>         Metaspace::ergo_initialize().
>>>>          >>
>>>>          >>
>>>>          >> Thanks,
>>>>          >>
>>>>          >> Yasumasa
>>>>          >>
>>>>
>>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
Hi all,

I added a testcase for this issue in new webrev:

  http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/


Thanks,

Yasumasa



2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:

> Hi all,
>
> I uploaded webrev for this issue against jdk10/hs.
> Could you review it?
>
>   http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>
>
> I cannot access JPRT. So I need a sponsor.
>
>
> Thanks,
>
> Yasumasa
>
>
>
> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>> Thank Yasumasa and Stefan for the responses.
>>
>> Good to know that the patch is not blocked due to breaking some internal
>> invariants/assumptions, but just due to its P5 status.
>> Is it possible to push it to P4?
>>
>> -Man
>>
>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga <[hidden email]>
>> wrote:
>>>
>>> Hi,
>>>
>>> (CC'ed hotspot-runtime-dev)
>>>
>>>> I think the reason is that this bug is a P5. The compressed class space
>>>> belongs to the runtime code, so you might get more traction for this on the
>>>> hotspot-runtime-dev list.
>>>
>>>
>>> I will send review request against jdk10/master or jdk10/hs after repos
>>> are opened.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>
>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>
>>>> Hi Man,
>>>>
>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>
>>>>> Hi Yasumasa, Stefan,
>>>>>
>>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>>> years? This patch could really save us from some annoying issues since we
>>>>> are automatically monitoring hsperfdata counters.
>>>>
>>>>
>>>> I think the reason is that this bug is a P5. The compressed class space
>>>> belongs to the runtime code, so you might get more traction for this on the
>>>> hotspot-runtime-dev list.
>>>>
>>>> StefanK
>>>>
>>>>>
>>>>> -Man
>>>>>
>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>> <mailto:[hidden email]>> wrote:
>>>>>
>>>>>     Hi all,
>>>>>
>>>>>     I wonder if there is any recent update on the patch for JDK-8087291.
>>>>>     Is it possible to push this patch into JDK9? Except for its low
>>>>>     priority (P5),
>>>>>     is there any complication that prevents this patch getting approved
>>>>>     (for example, some JVM logic requires CompressedClassSpaceSize to be
>>>>>     1GB by default)?
>>>>>
>>>>>     I work in the Java Platform Team at Google. We have encountered
>>>>>     annoying issues that the hsperfdata counter
>>>>>     "sun_gc_metaspace_maxCapacity" reporting
>>>>>     a too large value (about 1GB) even if user sets
>>>>>     -XX:MaxMetaspaceSize=100m, as well as GC log shows the confusing 1GB
>>>>>     memory reserved by metaspace,
>>>>>     regardless of MaxMetaspaceSize value. The root cause for these
>>>>>     issues is that CompressedClassSpaceSize is not automatically capped
>>>>>     by MaxMetaspaceSize
>>>>>     during VM initialization, and this patch seems fix the root cause.
>>>>>     (I'm aware that even after this patch, the reserved size could still
>>>>>     be up to 2*MaxMetaspaceSize,
>>>>>     but it is better than the current situation.)
>>>>>
>>>>>     Thanks,
>>>>>     Man
>>>>>
>>>>>     On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>
>>>>>         Thank you for your comment!
>>>>>          > Try running a debug JVM with your patch with this command
>>>>> line.
>>>>>          >
>>>>>          > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>         Sorry, I've fixed it and uploaded webrev:
>>>>>         http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>         <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>         It works on fastdebug VM.
>>>>>         Please review again.
>>>>>
>>>>>         Thanks,
>>>>>         Yasumasa
>>>>>
>>>>>         On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>          > Yasumasa,
>>>>>          >
>>>>>          > Try running a debug JVM with your patch with this command
>>>>> line.
>>>>>          >
>>>>>          > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>          >
>>>>>          > On a linux system I get this when I build with your patch.
>>>>>          >
>>>>>          >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>          >> # To suppress the following error report, specify this
>>>>> argument
>>>>>          >> # after -XX: or in .hotspotrc:
>>>>>         SuppressErrorAt=/metaspace.cpp:2324
>>>>>          >> #
>>>>>          >> # A fatal error has been detected by the Java Runtime
>>>>>         Environment:
>>>>>          >> #
>>>>>          >> #  Internal Error
>>>>>          >>
>>>>>
>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>          >> pid=19099, tid=0x00007ff4b9b92700
>>>>>          >> #  assert(size > MediumChunk || size > ClassMediumChunk)
>>>>>         failed: Not a
>>>>>          >> humongous chunk
>>>>>          >> #
>>>>>          >
>>>>>          >
>>>>>          > Jon
>>>>>          >
>>>>>          >
>>>>>          > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>          >> I want to continue to discuss about CompressedClassSpace and
>>>>>         MaxMetaspace in this (RFR) thread.
>>>>>          >>
>>>>>          >>
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>
>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>          >>>> Should I resize CompressedClassSpaceSize than to show
>>>>>         error message?
>>>>>          >>> If you add slightly better heuristics for the setup of the
>>>>>         CompressedClassSpaceSize flag, for example lowering the
>>>>>         CompressedClassSpaceSize when MaxMetaspaceSize is set, then it
>>>>>         might be less likely that you'll hit the OutOfMemoryError when
>>>>>         the system is set up with strict overcommit settings.
>>>>>          >>
>>>>>          >> I've uploaded new webrev:
>>>>>          >> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>         <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>          >>
>>>>>          >> This patch checkes MaxMetaspaceSize,
>>>>>         CompressedClassSpaceSize, and
>>>>>          >> InitialBootClassLoaderMetaspaceSize.
>>>>>          >>
>>>>>          >> I add to check CompressedClassSpaceSize in
>>>>>         Arguments::set_use_compressed_klass_ptrs().
>>>>>          >> If InitialBootClassLoaderMetaspaceSize is greater than
>>>>>         MaxMetaspaceSize,
>>>>>          >> VM will fail with error message.
>>>>>          >>
>>>>>          >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>>         MaxMetaspaceSize
>>>>>          >> when UseCompressedClassPointers is not set in
>>>>>         Metaspace::ergo_initialize().
>>>>>          >>
>>>>>          >>
>>>>>          >> Thanks,
>>>>>          >>
>>>>>          >> Yasumasa
>>>>>          >>
>>>>>
>>>>>
>>
Reply | Threaded
Open this post in threaded view
|

PING: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
PING:

Could you review and sponsor it?

>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/


Thanks,

Yasumasa


On 2017/09/27 12:05, Yasumasa Suenaga wrote:

> Hi all,
>
> I added a testcase for this issue in new webrev:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>
>
> Thanks,
>
> Yasumasa
>
>
>
> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>> Hi all,
>>
>> I uploaded webrev for this issue against jdk10/hs.
>> Could you review it?
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>
>>
>> I cannot access JPRT. So I need a sponsor.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>> Thank Yasumasa and Stefan for the responses.
>>>
>>> Good to know that the patch is not blocked due to breaking some internal
>>> invariants/assumptions, but just due to its P5 status.
>>> Is it possible to push it to P4?
>>>
>>> -Man
>>>
>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga <[hidden email]>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> (CC'ed hotspot-runtime-dev)
>>>>
>>>>> I think the reason is that this bug is a P5. The compressed class space
>>>>> belongs to the runtime code, so you might get more traction for this on the
>>>>> hotspot-runtime-dev list.
>>>>
>>>>
>>>> I will send review request against jdk10/master or jdk10/hs after repos
>>>> are opened.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>
>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>
>>>>> Hi Man,
>>>>>
>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>
>>>>>> Hi Yasumasa, Stefan,
>>>>>>
>>>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>>>> years? This patch could really save us from some annoying issues since we
>>>>>> are automatically monitoring hsperfdata counters.
>>>>>
>>>>>
>>>>> I think the reason is that this bug is a P5. The compressed class space
>>>>> belongs to the runtime code, so you might get more traction for this on the
>>>>> hotspot-runtime-dev list.
>>>>>
>>>>> StefanK
>>>>>
>>>>>>
>>>>>> -Man
>>>>>>
>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>
>>>>>>      Hi all,
>>>>>>
>>>>>>      I wonder if there is any recent update on the patch for JDK-8087291.
>>>>>>      Is it possible to push this patch into JDK9? Except for its low
>>>>>>      priority (P5),
>>>>>>      is there any complication that prevents this patch getting approved
>>>>>>      (for example, some JVM logic requires CompressedClassSpaceSize to be
>>>>>>      1GB by default)?
>>>>>>
>>>>>>      I work in the Java Platform Team at Google. We have encountered
>>>>>>      annoying issues that the hsperfdata counter
>>>>>>      "sun_gc_metaspace_maxCapacity" reporting
>>>>>>      a too large value (about 1GB) even if user sets
>>>>>>      -XX:MaxMetaspaceSize=100m, as well as GC log shows the confusing 1GB
>>>>>>      memory reserved by metaspace,
>>>>>>      regardless of MaxMetaspaceSize value. The root cause for these
>>>>>>      issues is that CompressedClassSpaceSize is not automatically capped
>>>>>>      by MaxMetaspaceSize
>>>>>>      during VM initialization, and this patch seems fix the root cause.
>>>>>>      (I'm aware that even after this patch, the reserved size could still
>>>>>>      be up to 2*MaxMetaspaceSize,
>>>>>>      but it is better than the current situation.)
>>>>>>
>>>>>>      Thanks,
>>>>>>      Man
>>>>>>
>>>>>>      On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>
>>>>>>          Thank you for your comment!
>>>>>>           > Try running a debug JVM with your patch with this command
>>>>>> line.
>>>>>>           >
>>>>>>           > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>          Sorry, I've fixed it and uploaded webrev:
>>>>>>          http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>          <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>          It works on fastdebug VM.
>>>>>>          Please review again.
>>>>>>
>>>>>>          Thanks,
>>>>>>          Yasumasa
>>>>>>
>>>>>>          On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>           > Yasumasa,
>>>>>>           >
>>>>>>           > Try running a debug JVM with your patch with this command
>>>>>> line.
>>>>>>           >
>>>>>>           > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>           >
>>>>>>           > On a linux system I get this when I build with your patch.
>>>>>>           >
>>>>>>           >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>           >> # To suppress the following error report, specify this
>>>>>> argument
>>>>>>           >> # after -XX: or in .hotspotrc:
>>>>>>          SuppressErrorAt=/metaspace.cpp:2324
>>>>>>           >> #
>>>>>>           >> # A fatal error has been detected by the Java Runtime
>>>>>>          Environment:
>>>>>>           >> #
>>>>>>           >> #  Internal Error
>>>>>>           >>
>>>>>>
>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>           >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>           >> #  assert(size > MediumChunk || size > ClassMediumChunk)
>>>>>>          failed: Not a
>>>>>>           >> humongous chunk
>>>>>>           >> #
>>>>>>           >
>>>>>>           >
>>>>>>           > Jon
>>>>>>           >
>>>>>>           >
>>>>>>           > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>           >> I want to continue to discuss about CompressedClassSpace and
>>>>>>          MaxMetaspace in this (RFR) thread.
>>>>>>           >>
>>>>>>           >>
>>>>>>
>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>
>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>           >>>> Should I resize CompressedClassSpaceSize than to show
>>>>>>          error message?
>>>>>>           >>> If you add slightly better heuristics for the setup of the
>>>>>>          CompressedClassSpaceSize flag, for example lowering the
>>>>>>          CompressedClassSpaceSize when MaxMetaspaceSize is set, then it
>>>>>>          might be less likely that you'll hit the OutOfMemoryError when
>>>>>>          the system is set up with strict overcommit settings.
>>>>>>           >>
>>>>>>           >> I've uploaded new webrev:
>>>>>>           >> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>          <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>           >>
>>>>>>           >> This patch checkes MaxMetaspaceSize,
>>>>>>          CompressedClassSpaceSize, and
>>>>>>           >> InitialBootClassLoaderMetaspaceSize.
>>>>>>           >>
>>>>>>           >> I add to check CompressedClassSpaceSize in
>>>>>>          Arguments::set_use_compressed_klass_ptrs().
>>>>>>           >> If InitialBootClassLoaderMetaspaceSize is greater than
>>>>>>          MaxMetaspaceSize,
>>>>>>           >> VM will fail with error message.
>>>>>>           >>
>>>>>>           >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>>>          MaxMetaspaceSize
>>>>>>           >> when UseCompressedClassPointers is not set in
>>>>>>          Metaspace::ergo_initialize().
>>>>>>           >>
>>>>>>           >>
>>>>>>           >> Thanks,
>>>>>>           >>
>>>>>>           >> Yasumasa
>>>>>>           >>
>>>>>>
>>>>>>
>>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

coleen.phillimore
In reply to this post by Yasumasa Suenaga-4
Hi, this looks reasonable but I would prefer that the code in
arguments.cpp call something in metaspace.cpp, like
check_metaspace_sizes() so that you don't have to tell arguments what
the MediumChunk size is.

I will sponsor this for you.

thanks,
Coleen

On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:

> Hi all,
>
> I added a testcase for this issue in new webrev:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>
>
> Thanks,
>
> Yasumasa
>
>
>
> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>> Hi all,
>>
>> I uploaded webrev for this issue against jdk10/hs.
>> Could you review it?
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>
>>
>> I cannot access JPRT. So I need a sponsor.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>> Thank Yasumasa and Stefan for the responses.
>>>
>>> Good to know that the patch is not blocked due to breaking some internal
>>> invariants/assumptions, but just due to its P5 status.
>>> Is it possible to push it to P4?
>>>
>>> -Man
>>>
>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga <[hidden email]>
>>> wrote:
>>>> Hi,
>>>>
>>>> (CC'ed hotspot-runtime-dev)
>>>>
>>>>> I think the reason is that this bug is a P5. The compressed class space
>>>>> belongs to the runtime code, so you might get more traction for this on the
>>>>> hotspot-runtime-dev list.
>>>>
>>>> I will send review request against jdk10/master or jdk10/hs after repos
>>>> are opened.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>
>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>> Hi Man,
>>>>>
>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>> Hi Yasumasa, Stefan,
>>>>>>
>>>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>>>> years? This patch could really save us from some annoying issues since we
>>>>>> are automatically monitoring hsperfdata counters.
>>>>>
>>>>> I think the reason is that this bug is a P5. The compressed class space
>>>>> belongs to the runtime code, so you might get more traction for this on the
>>>>> hotspot-runtime-dev list.
>>>>>
>>>>> StefanK
>>>>>
>>>>>> -Man
>>>>>>
>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>
>>>>>>      Hi all,
>>>>>>
>>>>>>      I wonder if there is any recent update on the patch for JDK-8087291.
>>>>>>      Is it possible to push this patch into JDK9? Except for its low
>>>>>>      priority (P5),
>>>>>>      is there any complication that prevents this patch getting approved
>>>>>>      (for example, some JVM logic requires CompressedClassSpaceSize to be
>>>>>>      1GB by default)?
>>>>>>
>>>>>>      I work in the Java Platform Team at Google. We have encountered
>>>>>>      annoying issues that the hsperfdata counter
>>>>>>      "sun_gc_metaspace_maxCapacity" reporting
>>>>>>      a too large value (about 1GB) even if user sets
>>>>>>      -XX:MaxMetaspaceSize=100m, as well as GC log shows the confusing 1GB
>>>>>>      memory reserved by metaspace,
>>>>>>      regardless of MaxMetaspaceSize value. The root cause for these
>>>>>>      issues is that CompressedClassSpaceSize is not automatically capped
>>>>>>      by MaxMetaspaceSize
>>>>>>      during VM initialization, and this patch seems fix the root cause.
>>>>>>      (I'm aware that even after this patch, the reserved size could still
>>>>>>      be up to 2*MaxMetaspaceSize,
>>>>>>      but it is better than the current situation.)
>>>>>>
>>>>>>      Thanks,
>>>>>>      Man
>>>>>>
>>>>>>      On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>
>>>>>>          Thank you for your comment!
>>>>>>           > Try running a debug JVM with your patch with this command
>>>>>> line.
>>>>>>           >
>>>>>>           > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>          Sorry, I've fixed it and uploaded webrev:
>>>>>>          http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>          <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>          It works on fastdebug VM.
>>>>>>          Please review again.
>>>>>>
>>>>>>          Thanks,
>>>>>>          Yasumasa
>>>>>>
>>>>>>          On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>           > Yasumasa,
>>>>>>           >
>>>>>>           > Try running a debug JVM with your patch with this command
>>>>>> line.
>>>>>>           >
>>>>>>           > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>           >
>>>>>>           > On a linux system I get this when I build with your patch.
>>>>>>           >
>>>>>>           >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>           >> # To suppress the following error report, specify this
>>>>>> argument
>>>>>>           >> # after -XX: or in .hotspotrc:
>>>>>>          SuppressErrorAt=/metaspace.cpp:2324
>>>>>>           >> #
>>>>>>           >> # A fatal error has been detected by the Java Runtime
>>>>>>          Environment:
>>>>>>           >> #
>>>>>>           >> #  Internal Error
>>>>>>           >>
>>>>>>
>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>           >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>           >> #  assert(size > MediumChunk || size > ClassMediumChunk)
>>>>>>          failed: Not a
>>>>>>           >> humongous chunk
>>>>>>           >> #
>>>>>>           >
>>>>>>           >
>>>>>>           > Jon
>>>>>>           >
>>>>>>           >
>>>>>>           > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>           >> I want to continue to discuss about CompressedClassSpace and
>>>>>>          MaxMetaspace in this (RFR) thread.
>>>>>>           >>
>>>>>>           >>
>>>>>>
>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>
>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>           >>>> Should I resize CompressedClassSpaceSize than to show
>>>>>>          error message?
>>>>>>           >>> If you add slightly better heuristics for the setup of the
>>>>>>          CompressedClassSpaceSize flag, for example lowering the
>>>>>>          CompressedClassSpaceSize when MaxMetaspaceSize is set, then it
>>>>>>          might be less likely that you'll hit the OutOfMemoryError when
>>>>>>          the system is set up with strict overcommit settings.
>>>>>>           >>
>>>>>>           >> I've uploaded new webrev:
>>>>>>           >> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>          <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>           >>
>>>>>>           >> This patch checkes MaxMetaspaceSize,
>>>>>>          CompressedClassSpaceSize, and
>>>>>>           >> InitialBootClassLoaderMetaspaceSize.
>>>>>>           >>
>>>>>>           >> I add to check CompressedClassSpaceSize in
>>>>>>          Arguments::set_use_compressed_klass_ptrs().
>>>>>>           >> If InitialBootClassLoaderMetaspaceSize is greater than
>>>>>>          MaxMetaspaceSize,
>>>>>>           >> VM will fail with error message.
>>>>>>           >>
>>>>>>           >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>>>          MaxMetaspaceSize
>>>>>>           >> when UseCompressedClassPointers is not set in
>>>>>>          Metaspace::ergo_initialize().
>>>>>>           >>
>>>>>>           >>
>>>>>>           >> Thanks,
>>>>>>           >>
>>>>>>           >> Yasumasa
>>>>>>           >>
>>>>>>
>>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Man Cao
Thanks for you both updating and sponsoring the webrev!
We plan to back-port it to our JDK9 once it is submitted.

-Man

On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:

> Hi, this looks reasonable but I would prefer that the code in
> arguments.cpp call something in metaspace.cpp, like check_metaspace_sizes()
> so that you don't have to tell arguments what the MediumChunk size is.
>
> I will sponsor this for you.
>
> thanks,
> Coleen
>
>
> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>
>> Hi all,
>>
>> I added a testcase for this issue in new webrev:
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>
>>> Hi all,
>>>
>>> I uploaded webrev for this issue against jdk10/hs.
>>> Could you review it?
>>>
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>
>>>
>>> I cannot access JPRT. So I need a sponsor.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>
>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>
>>>> Thank Yasumasa and Stefan for the responses.
>>>>
>>>> Good to know that the patch is not blocked due to breaking some internal
>>>> invariants/assumptions, but just due to its P5 status.
>>>> Is it possible to push it to P4?
>>>>
>>>> -Man
>>>>
>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga <[hidden email]>
>>>> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> (CC'ed hotspot-runtime-dev)
>>>>>
>>>>> I think the reason is that this bug is a P5. The compressed class space
>>>>>> belongs to the runtime code, so you might get more traction for this
>>>>>> on the
>>>>>> hotspot-runtime-dev list.
>>>>>>
>>>>>
>>>>> I will send review request against jdk10/master or jdk10/hs after repos
>>>>> are opened.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>
>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>
>>>>>> Hi Man,
>>>>>>
>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>
>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>
>>>>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>>>>> years? This patch could really save us from some annoying issues
>>>>>>> since we
>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>
>>>>>>
>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>> space
>>>>>> belongs to the runtime code, so you might get more traction for this
>>>>>> on the
>>>>>> hotspot-runtime-dev list.
>>>>>>
>>>>>> StefanK
>>>>>>
>>>>>> -Man
>>>>>>>
>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>
>>>>>>>      Hi all,
>>>>>>>
>>>>>>>      I wonder if there is any recent update on the patch for
>>>>>>> JDK-8087291.
>>>>>>>      Is it possible to push this patch into JDK9? Except for its low
>>>>>>>      priority (P5),
>>>>>>>      is there any complication that prevents this patch getting
>>>>>>> approved
>>>>>>>      (for example, some JVM logic requires CompressedClassSpaceSize
>>>>>>> to be
>>>>>>>      1GB by default)?
>>>>>>>
>>>>>>>      I work in the Java Platform Team at Google. We have encountered
>>>>>>>      annoying issues that the hsperfdata counter
>>>>>>>      "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>      a too large value (about 1GB) even if user sets
>>>>>>>      -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>> confusing 1GB
>>>>>>>      memory reserved by metaspace,
>>>>>>>      regardless of MaxMetaspaceSize value. The root cause for these
>>>>>>>      issues is that CompressedClassSpaceSize is not automatically
>>>>>>> capped
>>>>>>>      by MaxMetaspaceSize
>>>>>>>      during VM initialization, and this patch seems fix the root
>>>>>>> cause.
>>>>>>>      (I'm aware that even after this patch, the reserved size could
>>>>>>> still
>>>>>>>      be up to 2*MaxMetaspaceSize,
>>>>>>>      but it is better than the current situation.)
>>>>>>>
>>>>>>>      Thanks,
>>>>>>>      Man
>>>>>>>
>>>>>>>      On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>>          Thank you for your comment!
>>>>>>>           > Try running a debug JVM with your patch with this command
>>>>>>> line.
>>>>>>>           >
>>>>>>>           > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>          Sorry, I've fixed it and uploaded webrev:
>>>>>>>          http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>          <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02
>>>>>>> />
>>>>>>>          It works on fastdebug VM.
>>>>>>>          Please review again.
>>>>>>>
>>>>>>>          Thanks,
>>>>>>>          Yasumasa
>>>>>>>
>>>>>>>          On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>           > Yasumasa,
>>>>>>>           >
>>>>>>>           > Try running a debug JVM with your patch with this command
>>>>>>> line.
>>>>>>>           >
>>>>>>>           > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>           >
>>>>>>>           > On a linux system I get this when I build with your
>>>>>>> patch.
>>>>>>>           >
>>>>>>>           >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>           >> # To suppress the following error report, specify this
>>>>>>> argument
>>>>>>>           >> # after -XX: or in .hotspotrc:
>>>>>>>          SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>           >> #
>>>>>>>           >> # A fatal error has been detected by the Java Runtime
>>>>>>>          Environment:
>>>>>>>           >> #
>>>>>>>           >> #  Internal Error
>>>>>>>           >>
>>>>>>>
>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/
>>>>>>> metaspace.cpp:2324),
>>>>>>>           >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>           >> #  assert(size > MediumChunk || size > ClassMediumChunk)
>>>>>>>          failed: Not a
>>>>>>>           >> humongous chunk
>>>>>>>           >> #
>>>>>>>           >
>>>>>>>           >
>>>>>>>           > Jon
>>>>>>>           >
>>>>>>>           >
>>>>>>>           > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>           >> I want to continue to discuss about
>>>>>>> CompressedClassSpace and
>>>>>>>          MaxMetaspace in this (RFR) thread.
>>>>>>>           >>
>>>>>>>           >>
>>>>>>>
>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-J
>>>>>>> une/013873.html
>>>>>>>
>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-
>>>>>>> June/013873.html>
>>>>>>>           >>>> Should I resize CompressedClassSpaceSize than to show
>>>>>>>          error message?
>>>>>>>           >>> If you add slightly better heuristics for the setup of
>>>>>>> the
>>>>>>>          CompressedClassSpaceSize flag, for example lowering the
>>>>>>>          CompressedClassSpaceSize when MaxMetaspaceSize is set, then
>>>>>>> it
>>>>>>>          might be less likely that you'll hit the OutOfMemoryError
>>>>>>> when
>>>>>>>          the system is set up with strict overcommit settings.
>>>>>>>           >>
>>>>>>>           >> I've uploaded new webrev:
>>>>>>>           >> http://cr.openjdk.java.net/~ys
>>>>>>> uenaga/JDK-8087291/webrev.01/
>>>>>>>          <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01
>>>>>>> />
>>>>>>>           >>
>>>>>>>           >> This patch checkes MaxMetaspaceSize,
>>>>>>>          CompressedClassSpaceSize, and
>>>>>>>           >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>           >>
>>>>>>>           >> I add to check CompressedClassSpaceSize in
>>>>>>>          Arguments::set_use_compressed_klass_ptrs().
>>>>>>>           >> If InitialBootClassLoaderMetaspaceSize is greater than
>>>>>>>          MaxMetaspaceSize,
>>>>>>>           >> VM will fail with error message.
>>>>>>>           >>
>>>>>>>           >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>>>>          MaxMetaspaceSize
>>>>>>>           >> when UseCompressedClassPointers is not set in
>>>>>>>          Metaspace::ergo_initialize().
>>>>>>>           >>
>>>>>>>           >>
>>>>>>>           >> Thanks,
>>>>>>>           >>
>>>>>>>           >> Yasumasa
>>>>>>>           >>
>>>>>>>
>>>>>>>
>>>>>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
In reply to this post by Yasumasa Suenaga-4
Hi Coleen,

>> I will sponsor this for you.

Thanks!


>> Hi, this looks reasonable but I would prefer that the code in
>> arguments.cpp call something in metaspace.cpp, like check_metaspace_sizes()
>> so that you don't have to tell arguments what the MediumChunk size is.

I added new function calculate_min_metaspace_size() in metaspace.cpp
to get minimum metaspace size, and uses return value from this
function in metaspace initialization and metaspace size check.

  http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/

Could you review again?


Thanks,

Yasumasa


2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:

> Thanks for you both updating and sponsoring the webrev!
> We plan to back-port it to our JDK9 once it is submitted.
>
> -Man
>
> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>
>> Hi, this looks reasonable but I would prefer that the code in
>> arguments.cpp call something in metaspace.cpp, like check_metaspace_sizes()
>> so that you don't have to tell arguments what the MediumChunk size is.
>>
>> I will sponsor this for you.
>>
>> thanks,
>> Coleen
>>
>>
>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>
>>> Hi all,
>>>
>>> I added a testcase for this issue in new webrev:
>>>
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>
>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>
>>>> Hi all,
>>>>
>>>> I uploaded webrev for this issue against jdk10/hs.
>>>> Could you review it?
>>>>
>>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>
>>>>
>>>> I cannot access JPRT. So I need a sponsor.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>
>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>
>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>
>>>>> Good to know that the patch is not blocked due to breaking some
>>>>> internal
>>>>> invariants/assumptions, but just due to its P5 status.
>>>>> Is it possible to push it to P4?
>>>>>
>>>>> -Man
>>>>>
>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga <[hidden email]>
>>>>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>
>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>> space
>>>>>>> belongs to the runtime code, so you might get more traction for this
>>>>>>> on the
>>>>>>> hotspot-runtime-dev list.
>>>>>>
>>>>>>
>>>>>> I will send review request against jdk10/master or jdk10/hs after
>>>>>> repos
>>>>>> are opened.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>
>>>>>>> Hi Man,
>>>>>>>
>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>
>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>
>>>>>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>>>>>> years? This patch could really save us from some annoying issues
>>>>>>>> since we
>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>
>>>>>>>
>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>> space
>>>>>>> belongs to the runtime code, so you might get more traction for this
>>>>>>> on the
>>>>>>> hotspot-runtime-dev list.
>>>>>>>
>>>>>>> StefanK
>>>>>>>
>>>>>>>> -Man
>>>>>>>>
>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>
>>>>>>>>      Hi all,
>>>>>>>>
>>>>>>>>      I wonder if there is any recent update on the patch for
>>>>>>>> JDK-8087291.
>>>>>>>>      Is it possible to push this patch into JDK9? Except for its low
>>>>>>>>      priority (P5),
>>>>>>>>      is there any complication that prevents this patch getting
>>>>>>>> approved
>>>>>>>>      (for example, some JVM logic requires CompressedClassSpaceSize
>>>>>>>> to be
>>>>>>>>      1GB by default)?
>>>>>>>>
>>>>>>>>      I work in the Java Platform Team at Google. We have encountered
>>>>>>>>      annoying issues that the hsperfdata counter
>>>>>>>>      "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>      a too large value (about 1GB) even if user sets
>>>>>>>>      -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>> confusing 1GB
>>>>>>>>      memory reserved by metaspace,
>>>>>>>>      regardless of MaxMetaspaceSize value. The root cause for these
>>>>>>>>      issues is that CompressedClassSpaceSize is not automatically
>>>>>>>> capped
>>>>>>>>      by MaxMetaspaceSize
>>>>>>>>      during VM initialization, and this patch seems fix the root
>>>>>>>> cause.
>>>>>>>>      (I'm aware that even after this patch, the reserved size could
>>>>>>>> still
>>>>>>>>      be up to 2*MaxMetaspaceSize,
>>>>>>>>      but it is better than the current situation.)
>>>>>>>>
>>>>>>>>      Thanks,
>>>>>>>>      Man
>>>>>>>>
>>>>>>>>      On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>
>>>>>>>>          Thank you for your comment!
>>>>>>>>           > Try running a debug JVM with your patch with this
>>>>>>>> command
>>>>>>>> line.
>>>>>>>>           >
>>>>>>>>           > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>          Sorry, I've fixed it and uploaded webrev:
>>>>>>>>          http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>
>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>          It works on fastdebug VM.
>>>>>>>>          Please review again.
>>>>>>>>
>>>>>>>>          Thanks,
>>>>>>>>          Yasumasa
>>>>>>>>
>>>>>>>>          On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>           > Yasumasa,
>>>>>>>>           >
>>>>>>>>           > Try running a debug JVM with your patch with this
>>>>>>>> command
>>>>>>>> line.
>>>>>>>>           >
>>>>>>>>           > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>           >
>>>>>>>>           > On a linux system I get this when I build with your
>>>>>>>> patch.
>>>>>>>>           >
>>>>>>>>           >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>           >> # To suppress the following error report, specify this
>>>>>>>> argument
>>>>>>>>           >> # after -XX: or in .hotspotrc:
>>>>>>>>          SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>           >> #
>>>>>>>>           >> # A fatal error has been detected by the Java Runtime
>>>>>>>>          Environment:
>>>>>>>>           >> #
>>>>>>>>           >> #  Internal Error
>>>>>>>>           >>
>>>>>>>>
>>>>>>>>
>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>           >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>           >> #  assert(size > MediumChunk || size >
>>>>>>>> ClassMediumChunk)
>>>>>>>>          failed: Not a
>>>>>>>>           >> humongous chunk
>>>>>>>>           >> #
>>>>>>>>           >
>>>>>>>>           >
>>>>>>>>           > Jon
>>>>>>>>           >
>>>>>>>>           >
>>>>>>>>           > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>           >> I want to continue to discuss about
>>>>>>>> CompressedClassSpace and
>>>>>>>>          MaxMetaspace in this (RFR) thread.
>>>>>>>>           >>
>>>>>>>>           >>
>>>>>>>>
>>>>>>>>
>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>
>>>>>>>>
>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>           >>>> Should I resize CompressedClassSpaceSize than to show
>>>>>>>>          error message?
>>>>>>>>           >>> If you add slightly better heuristics for the setup of
>>>>>>>> the
>>>>>>>>          CompressedClassSpaceSize flag, for example lowering the
>>>>>>>>          CompressedClassSpaceSize when MaxMetaspaceSize is set, then
>>>>>>>> it
>>>>>>>>          might be less likely that you'll hit the OutOfMemoryError
>>>>>>>> when
>>>>>>>>          the system is set up with strict overcommit settings.
>>>>>>>>           >>
>>>>>>>>           >> I've uploaded new webrev:
>>>>>>>>           >>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>
>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>           >>
>>>>>>>>           >> This patch checkes MaxMetaspaceSize,
>>>>>>>>          CompressedClassSpaceSize, and
>>>>>>>>           >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>           >>
>>>>>>>>           >> I add to check CompressedClassSpaceSize in
>>>>>>>>          Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>           >> If InitialBootClassLoaderMetaspaceSize is greater than
>>>>>>>>          MaxMetaspaceSize,
>>>>>>>>           >> VM will fail with error message.
>>>>>>>>           >>
>>>>>>>>           >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>>>>>          MaxMetaspaceSize
>>>>>>>>           >> when UseCompressedClassPointers is not set in
>>>>>>>>          Metaspace::ergo_initialize().
>>>>>>>>           >>
>>>>>>>>           >>
>>>>>>>>           >> Thanks,
>>>>>>>>           >>
>>>>>>>>           >> Yasumasa
>>>>>>>>           >>
>>>>>>>>
>>>>>>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

coleen.phillimore
Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only
needs one reviewer, unless there were more?  Please send me the result
of hg export.
thanks,
Coleen

On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:

> Hi Coleen,
>
>>> I will sponsor this for you.
> Thanks!
>
>
>>> Hi, this looks reasonable but I would prefer that the code in
>>> arguments.cpp call something in metaspace.cpp, like check_metaspace_sizes()
>>> so that you don't have to tell arguments what the MediumChunk size is.
> I added new function calculate_min_metaspace_size() in metaspace.cpp
> to get minimum metaspace size, and uses return value from this
> function in metaspace initialization and metaspace size check.
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>
> Could you review again?
>
>
> Thanks,
>
> Yasumasa
>
>
> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>> Thanks for you both updating and sponsoring the webrev!
>> We plan to back-port it to our JDK9 once it is submitted.
>>
>> -Man
>>
>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>> Hi, this looks reasonable but I would prefer that the code in
>>> arguments.cpp call something in metaspace.cpp, like check_metaspace_sizes()
>>> so that you don't have to tell arguments what the MediumChunk size is.
>>>
>>> I will sponsor this for you.
>>>
>>> thanks,
>>> Coleen
>>>
>>>
>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>> Hi all,
>>>>
>>>> I added a testcase for this issue in new webrev:
>>>>
>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>
>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>> Hi all,
>>>>>
>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>> Could you review it?
>>>>>
>>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>
>>>>>
>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>
>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>
>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>> internal
>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>> Is it possible to push it to P4?
>>>>>>
>>>>>> -Man
>>>>>>
>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga <[hidden email]>
>>>>>> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>
>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>> space
>>>>>>>> belongs to the runtime code, so you might get more traction for this
>>>>>>>> on the
>>>>>>>> hotspot-runtime-dev list.
>>>>>>>
>>>>>>> I will send review request against jdk10/master or jdk10/hs after
>>>>>>> repos
>>>>>>> are opened.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>> Hi Man,
>>>>>>>>
>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>
>>>>>>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>>>>>>> years? This patch could really save us from some annoying issues
>>>>>>>>> since we
>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>
>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>> space
>>>>>>>> belongs to the runtime code, so you might get more traction for this
>>>>>>>> on the
>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>
>>>>>>>> StefanK
>>>>>>>>
>>>>>>>>> -Man
>>>>>>>>>
>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>
>>>>>>>>>       Hi all,
>>>>>>>>>
>>>>>>>>>       I wonder if there is any recent update on the patch for
>>>>>>>>> JDK-8087291.
>>>>>>>>>       Is it possible to push this patch into JDK9? Except for its low
>>>>>>>>>       priority (P5),
>>>>>>>>>       is there any complication that prevents this patch getting
>>>>>>>>> approved
>>>>>>>>>       (for example, some JVM logic requires CompressedClassSpaceSize
>>>>>>>>> to be
>>>>>>>>>       1GB by default)?
>>>>>>>>>
>>>>>>>>>       I work in the Java Platform Team at Google. We have encountered
>>>>>>>>>       annoying issues that the hsperfdata counter
>>>>>>>>>       "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>       a too large value (about 1GB) even if user sets
>>>>>>>>>       -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>> confusing 1GB
>>>>>>>>>       memory reserved by metaspace,
>>>>>>>>>       regardless of MaxMetaspaceSize value. The root cause for these
>>>>>>>>>       issues is that CompressedClassSpaceSize is not automatically
>>>>>>>>> capped
>>>>>>>>>       by MaxMetaspaceSize
>>>>>>>>>       during VM initialization, and this patch seems fix the root
>>>>>>>>> cause.
>>>>>>>>>       (I'm aware that even after this patch, the reserved size could
>>>>>>>>> still
>>>>>>>>>       be up to 2*MaxMetaspaceSize,
>>>>>>>>>       but it is better than the current situation.)
>>>>>>>>>
>>>>>>>>>       Thanks,
>>>>>>>>>       Man
>>>>>>>>>
>>>>>>>>>       On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>
>>>>>>>>>           Thank you for your comment!
>>>>>>>>>            > Try running a debug JVM with your patch with this
>>>>>>>>> command
>>>>>>>>> line.
>>>>>>>>>            >
>>>>>>>>>            > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>           Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>           http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>
>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>           It works on fastdebug VM.
>>>>>>>>>           Please review again.
>>>>>>>>>
>>>>>>>>>           Thanks,
>>>>>>>>>           Yasumasa
>>>>>>>>>
>>>>>>>>>           On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>            > Yasumasa,
>>>>>>>>>            >
>>>>>>>>>            > Try running a debug JVM with your patch with this
>>>>>>>>> command
>>>>>>>>> line.
>>>>>>>>>            >
>>>>>>>>>            > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>            >
>>>>>>>>>            > On a linux system I get this when I build with your
>>>>>>>>> patch.
>>>>>>>>>            >
>>>>>>>>>            >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>            >> # To suppress the following error report, specify this
>>>>>>>>> argument
>>>>>>>>>            >> # after -XX: or in .hotspotrc:
>>>>>>>>>           SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>            >> #
>>>>>>>>>            >> # A fatal error has been detected by the Java Runtime
>>>>>>>>>           Environment:
>>>>>>>>>            >> #
>>>>>>>>>            >> #  Internal Error
>>>>>>>>>            >>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>            >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>            >> #  assert(size > MediumChunk || size >
>>>>>>>>> ClassMediumChunk)
>>>>>>>>>           failed: Not a
>>>>>>>>>            >> humongous chunk
>>>>>>>>>            >> #
>>>>>>>>>            >
>>>>>>>>>            >
>>>>>>>>>            > Jon
>>>>>>>>>            >
>>>>>>>>>            >
>>>>>>>>>            > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>            >> I want to continue to discuss about
>>>>>>>>> CompressedClassSpace and
>>>>>>>>>           MaxMetaspace in this (RFR) thread.
>>>>>>>>>            >>
>>>>>>>>>            >>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>            >>>> Should I resize CompressedClassSpaceSize than to show
>>>>>>>>>           error message?
>>>>>>>>>            >>> If you add slightly better heuristics for the setup of
>>>>>>>>> the
>>>>>>>>>           CompressedClassSpaceSize flag, for example lowering the
>>>>>>>>>           CompressedClassSpaceSize when MaxMetaspaceSize is set, then
>>>>>>>>> it
>>>>>>>>>           might be less likely that you'll hit the OutOfMemoryError
>>>>>>>>> when
>>>>>>>>>           the system is set up with strict overcommit settings.
>>>>>>>>>            >>
>>>>>>>>>            >> I've uploaded new webrev:
>>>>>>>>>            >>
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>
>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>            >>
>>>>>>>>>            >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>           CompressedClassSpaceSize, and
>>>>>>>>>            >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>            >>
>>>>>>>>>            >> I add to check CompressedClassSpaceSize in
>>>>>>>>>           Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>            >> If InitialBootClassLoaderMetaspaceSize is greater than
>>>>>>>>>           MaxMetaspaceSize,
>>>>>>>>>            >> VM will fail with error message.
>>>>>>>>>            >>
>>>>>>>>>            >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>>>>>>           MaxMetaspaceSize
>>>>>>>>>            >> when UseCompressedClassPointers is not set in
>>>>>>>>>           Metaspace::ergo_initialize().
>>>>>>>>>            >>
>>>>>>>>>            >>
>>>>>>>>>            >> Thanks,
>>>>>>>>>            >>
>>>>>>>>>            >> Yasumasa
>>>>>>>>>            >>
>>>>>>>>>
>>>>>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
Hi all,

This change was tried to push via JPRT, but it failed because
test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
was failed.
Also I heard the comment that the changes in arguments.cpp which set
ergonomic value to CompressedClassSpaceSize should be moved to
Metaspace::ergo_initialize().

I uploaded new webrev. Could you review again?
This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
test/hotspot/jtreg/runtime/SharedArchiveFile.

  http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/


Thanks,

Yasumasa



2017-10-10 21:02 GMT+09:00  <[hidden email]>:

> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only needs
> one reviewer, unless there were more?  Please send me the result of hg
> export.
> thanks,
> Coleen
>
>
> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>
>> Hi Coleen,
>>
>>>> I will sponsor this for you.
>>
>> Thanks!
>>
>>
>>>> Hi, this looks reasonable but I would prefer that the code in
>>>> arguments.cpp call something in metaspace.cpp, like
>>>> check_metaspace_sizes()
>>>> so that you don't have to tell arguments what the MediumChunk size is.
>>
>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>> to get minimum metaspace size, and uses return value from this
>> function in metaspace initialization and metaspace size check.
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>
>> Could you review again?
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>
>>> Thanks for you both updating and sponsoring the webrev!
>>> We plan to back-port it to our JDK9 once it is submitted.
>>>
>>> -Man
>>>
>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>>>
>>>> Hi, this looks reasonable but I would prefer that the code in
>>>> arguments.cpp call something in metaspace.cpp, like
>>>> check_metaspace_sizes()
>>>> so that you don't have to tell arguments what the MediumChunk size is.
>>>>
>>>> I will sponsor this for you.
>>>>
>>>> thanks,
>>>> Coleen
>>>>
>>>>
>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> I added a testcase for this issue in new webrev:
>>>>>
>>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>
>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>> Could you review it?
>>>>>>
>>>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>
>>>>>>
>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>
>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>
>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>> internal
>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>> Is it possible to push it to P4?
>>>>>>>
>>>>>>> -Man
>>>>>>>
>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>> <[hidden email]>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>
>>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>>> space
>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>> this
>>>>>>>>> on the
>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>
>>>>>>>>
>>>>>>>> I will send review request against jdk10/master or jdk10/hs after
>>>>>>>> repos
>>>>>>>> are opened.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>
>>>>>>>>> Hi Man,
>>>>>>>>>
>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>
>>>>>>>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>>>>>>>> years? This patch could really save us from some annoying issues
>>>>>>>>>> since we
>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>>> space
>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>> this
>>>>>>>>> on the
>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>
>>>>>>>>> StefanK
>>>>>>>>>
>>>>>>>>>> -Man
>>>>>>>>>>
>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>
>>>>>>>>>>       Hi all,
>>>>>>>>>>
>>>>>>>>>>       I wonder if there is any recent update on the patch for
>>>>>>>>>> JDK-8087291.
>>>>>>>>>>       Is it possible to push this patch into JDK9? Except for its
>>>>>>>>>> low
>>>>>>>>>>       priority (P5),
>>>>>>>>>>       is there any complication that prevents this patch getting
>>>>>>>>>> approved
>>>>>>>>>>       (for example, some JVM logic requires
>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>> to be
>>>>>>>>>>       1GB by default)?
>>>>>>>>>>
>>>>>>>>>>       I work in the Java Platform Team at Google. We have
>>>>>>>>>> encountered
>>>>>>>>>>       annoying issues that the hsperfdata counter
>>>>>>>>>>       "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>       a too large value (about 1GB) even if user sets
>>>>>>>>>>       -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>>> confusing 1GB
>>>>>>>>>>       memory reserved by metaspace,
>>>>>>>>>>       regardless of MaxMetaspaceSize value. The root cause for
>>>>>>>>>> these
>>>>>>>>>>       issues is that CompressedClassSpaceSize is not automatically
>>>>>>>>>> capped
>>>>>>>>>>       by MaxMetaspaceSize
>>>>>>>>>>       during VM initialization, and this patch seems fix the root
>>>>>>>>>> cause.
>>>>>>>>>>       (I'm aware that even after this patch, the reserved size
>>>>>>>>>> could
>>>>>>>>>> still
>>>>>>>>>>       be up to 2*MaxMetaspaceSize,
>>>>>>>>>>       but it is better than the current situation.)
>>>>>>>>>>
>>>>>>>>>>       Thanks,
>>>>>>>>>>       Man
>>>>>>>>>>
>>>>>>>>>>       On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>
>>>>>>>>>>           Thank you for your comment!
>>>>>>>>>>            > Try running a debug JVM with your patch with this
>>>>>>>>>> command
>>>>>>>>>> line.
>>>>>>>>>>            >
>>>>>>>>>>            > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>           Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>
>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>           It works on fastdebug VM.
>>>>>>>>>>           Please review again.
>>>>>>>>>>
>>>>>>>>>>           Thanks,
>>>>>>>>>>           Yasumasa
>>>>>>>>>>
>>>>>>>>>>           On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>            > Yasumasa,
>>>>>>>>>>            >
>>>>>>>>>>            > Try running a debug JVM with your patch with this
>>>>>>>>>> command
>>>>>>>>>> line.
>>>>>>>>>>            >
>>>>>>>>>>            > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>            >
>>>>>>>>>>            > On a linux system I get this when I build with your
>>>>>>>>>> patch.
>>>>>>>>>>            >
>>>>>>>>>>            >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>            >> # To suppress the following error report, specify
>>>>>>>>>> this
>>>>>>>>>> argument
>>>>>>>>>>            >> # after -XX: or in .hotspotrc:
>>>>>>>>>>           SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>            >> #
>>>>>>>>>>            >> # A fatal error has been detected by the Java
>>>>>>>>>> Runtime
>>>>>>>>>>           Environment:
>>>>>>>>>>            >> #
>>>>>>>>>>            >> #  Internal Error
>>>>>>>>>>            >>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>            >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>            >> #  assert(size > MediumChunk || size >
>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>           failed: Not a
>>>>>>>>>>            >> humongous chunk
>>>>>>>>>>            >> #
>>>>>>>>>>            >
>>>>>>>>>>            >
>>>>>>>>>>            > Jon
>>>>>>>>>>            >
>>>>>>>>>>            >
>>>>>>>>>>            > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>            >> I want to continue to discuss about
>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>           MaxMetaspace in this (RFR) thread.
>>>>>>>>>>            >>
>>>>>>>>>>            >>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>            >>>> Should I resize CompressedClassSpaceSize than to
>>>>>>>>>> show
>>>>>>>>>>           error message?
>>>>>>>>>>            >>> If you add slightly better heuristics for the setup
>>>>>>>>>> of
>>>>>>>>>> the
>>>>>>>>>>           CompressedClassSpaceSize flag, for example lowering the
>>>>>>>>>>           CompressedClassSpaceSize when MaxMetaspaceSize is set,
>>>>>>>>>> then
>>>>>>>>>> it
>>>>>>>>>>           might be less likely that you'll hit the
>>>>>>>>>> OutOfMemoryError
>>>>>>>>>> when
>>>>>>>>>>           the system is set up with strict overcommit settings.
>>>>>>>>>>            >>
>>>>>>>>>>            >> I've uploaded new webrev:
>>>>>>>>>>            >>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>
>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>            >>
>>>>>>>>>>            >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>           CompressedClassSpaceSize, and
>>>>>>>>>>            >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>            >>
>>>>>>>>>>            >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>           Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>            >> If InitialBootClassLoaderMetaspaceSize is greater
>>>>>>>>>> than
>>>>>>>>>>           MaxMetaspaceSize,
>>>>>>>>>>            >> VM will fail with error message.
>>>>>>>>>>            >>
>>>>>>>>>>            >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>>>>>>>           MaxMetaspaceSize
>>>>>>>>>>            >> when UseCompressedClassPointers is not set in
>>>>>>>>>>           Metaspace::ergo_initialize().
>>>>>>>>>>            >>
>>>>>>>>>>            >>
>>>>>>>>>>            >> Thanks,
>>>>>>>>>>            >>
>>>>>>>>>>            >> Yasumasa
>>>>>>>>>>            >>
>>>>>>>>>>
>>>>>>>>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

David Holmes
Hi Yasumasa,

On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:

> Hi all,
>
> This change was tried to push via JPRT, but it failed because
> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
> was failed.
> Also I heard the comment that the changes in arguments.cpp which set
> ergonomic value to CompressedClassSpaceSize should be moved to
> Metaspace::ergo_initialize().
>
> I uploaded new webrev. Could you review again?
> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
> test/hotspot/jtreg/runtime/SharedArchiveFile.
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/

I'll let Coleen cover the technical details, I'll just pick up a few
style nits:

3328   /* Initial virtual space size will be calculated at
global_initialize() */

Use // comment

3329   size_t min_metaspace_sz =
3330                    VIRTUALSPACEMULTIPLIER *
InitialBootClassLoaderMetaspaceSize;

            ^ should indent only to here

3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
3337                                       MaxMetaspaceSize -
min_metaspace_sz);
                            ^ should indent only to here

3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
3342
min_metaspace_sz);

                         ^ should indent only to here

You may have time to fix these in place before anyone else sees the
webrev :)

Thanks,
David
-----

>
> Thanks,
>
> Yasumasa
>
>
>
> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only needs
>> one reviewer, unless there were more?  Please send me the result of hg
>> export.
>> thanks,
>> Coleen
>>
>>
>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>
>>> Hi Coleen,
>>>
>>>>> I will sponsor this for you.
>>>
>>> Thanks!
>>>
>>>
>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>> check_metaspace_sizes()
>>>>> so that you don't have to tell arguments what the MediumChunk size is.
>>>
>>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>>> to get minimum metaspace size, and uses return value from this
>>> function in metaspace initialization and metaspace size check.
>>>
>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>
>>> Could you review again?
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>
>>>> Thanks for you both updating and sponsoring the webrev!
>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>
>>>> -Man
>>>>
>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>>>>
>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>> check_metaspace_sizes()
>>>>> so that you don't have to tell arguments what the MediumChunk size is.
>>>>>
>>>>> I will sponsor this for you.
>>>>>
>>>>> thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> I added a testcase for this issue in new webrev:
>>>>>>
>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>> Could you review it?
>>>>>>>
>>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>>
>>>>>>>
>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>
>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>
>>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>>> internal
>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>> Is it possible to push it to P4?
>>>>>>>>
>>>>>>>> -Man
>>>>>>>>
>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>> <[hidden email]>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>
>>>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>>>> space
>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>> this
>>>>>>>>>> on the
>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I will send review request against jdk10/master or jdk10/hs after
>>>>>>>>> repos
>>>>>>>>> are opened.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Man,
>>>>>>>>>>
>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>
>>>>>>>>>>> Do you have any thoughts on why this patch has been pending for 2+
>>>>>>>>>>> years? This patch could really save us from some annoying issues
>>>>>>>>>>> since we
>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>>>> space
>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>> this
>>>>>>>>>> on the
>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>
>>>>>>>>>> StefanK
>>>>>>>>>>
>>>>>>>>>>> -Man
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>        Hi all,
>>>>>>>>>>>
>>>>>>>>>>>        I wonder if there is any recent update on the patch for
>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>        Is it possible to push this patch into JDK9? Except for its
>>>>>>>>>>> low
>>>>>>>>>>>        priority (P5),
>>>>>>>>>>>        is there any complication that prevents this patch getting
>>>>>>>>>>> approved
>>>>>>>>>>>        (for example, some JVM logic requires
>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>> to be
>>>>>>>>>>>        1GB by default)?
>>>>>>>>>>>
>>>>>>>>>>>        I work in the Java Platform Team at Google. We have
>>>>>>>>>>> encountered
>>>>>>>>>>>        annoying issues that the hsperfdata counter
>>>>>>>>>>>        "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>        a too large value (about 1GB) even if user sets
>>>>>>>>>>>        -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>        memory reserved by metaspace,
>>>>>>>>>>>        regardless of MaxMetaspaceSize value. The root cause for
>>>>>>>>>>> these
>>>>>>>>>>>        issues is that CompressedClassSpaceSize is not automatically
>>>>>>>>>>> capped
>>>>>>>>>>>        by MaxMetaspaceSize
>>>>>>>>>>>        during VM initialization, and this patch seems fix the root
>>>>>>>>>>> cause.
>>>>>>>>>>>        (I'm aware that even after this patch, the reserved size
>>>>>>>>>>> could
>>>>>>>>>>> still
>>>>>>>>>>>        be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>        but it is better than the current situation.)
>>>>>>>>>>>
>>>>>>>>>>>        Thanks,
>>>>>>>>>>>        Man
>>>>>>>>>>>
>>>>>>>>>>>        On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>
>>>>>>>>>>>            Thank you for your comment!
>>>>>>>>>>>             > Try running a debug JVM with your patch with this
>>>>>>>>>>> command
>>>>>>>>>>> line.
>>>>>>>>>>>             >
>>>>>>>>>>>             > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>            Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>
>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>            It works on fastdebug VM.
>>>>>>>>>>>            Please review again.
>>>>>>>>>>>
>>>>>>>>>>>            Thanks,
>>>>>>>>>>>            Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>            On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>             > Yasumasa,
>>>>>>>>>>>             >
>>>>>>>>>>>             > Try running a debug JVM with your patch with this
>>>>>>>>>>> command
>>>>>>>>>>> line.
>>>>>>>>>>>             >
>>>>>>>>>>>             > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>             >
>>>>>>>>>>>             > On a linux system I get this when I build with your
>>>>>>>>>>> patch.
>>>>>>>>>>>             >
>>>>>>>>>>>             >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>             >> # To suppress the following error report, specify
>>>>>>>>>>> this
>>>>>>>>>>> argument
>>>>>>>>>>>             >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>            SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>             >> #
>>>>>>>>>>>             >> # A fatal error has been detected by the Java
>>>>>>>>>>> Runtime
>>>>>>>>>>>            Environment:
>>>>>>>>>>>             >> #
>>>>>>>>>>>             >> #  Internal Error
>>>>>>>>>>>             >>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>             >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>             >> #  assert(size > MediumChunk || size >
>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>            failed: Not a
>>>>>>>>>>>             >> humongous chunk
>>>>>>>>>>>             >> #
>>>>>>>>>>>             >
>>>>>>>>>>>             >
>>>>>>>>>>>             > Jon
>>>>>>>>>>>             >
>>>>>>>>>>>             >
>>>>>>>>>>>             > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>             >> I want to continue to discuss about
>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>            MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>             >>
>>>>>>>>>>>             >>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>             >>>> Should I resize CompressedClassSpaceSize than to
>>>>>>>>>>> show
>>>>>>>>>>>            error message?
>>>>>>>>>>>             >>> If you add slightly better heuristics for the setup
>>>>>>>>>>> of
>>>>>>>>>>> the
>>>>>>>>>>>            CompressedClassSpaceSize flag, for example lowering the
>>>>>>>>>>>            CompressedClassSpaceSize when MaxMetaspaceSize is set,
>>>>>>>>>>> then
>>>>>>>>>>> it
>>>>>>>>>>>            might be less likely that you'll hit the
>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>> when
>>>>>>>>>>>            the system is set up with strict overcommit settings.
>>>>>>>>>>>             >>
>>>>>>>>>>>             >> I've uploaded new webrev:
>>>>>>>>>>>             >>
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>
>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>             >>
>>>>>>>>>>>             >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>            CompressedClassSpaceSize, and
>>>>>>>>>>>             >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>             >>
>>>>>>>>>>>             >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>            Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>             >> If InitialBootClassLoaderMetaspaceSize is greater
>>>>>>>>>>> than
>>>>>>>>>>>            MaxMetaspaceSize,
>>>>>>>>>>>             >> VM will fail with error message.
>>>>>>>>>>>             >>
>>>>>>>>>>>             >> InitialBootClassLoaderMetaspaceSize will be set to
>>>>>>>>>>>            MaxMetaspaceSize
>>>>>>>>>>>             >> when UseCompressedClassPointers is not set in
>>>>>>>>>>>            Metaspace::ergo_initialize().
>>>>>>>>>>>             >>
>>>>>>>>>>>             >>
>>>>>>>>>>>             >> Thanks,
>>>>>>>>>>>             >>
>>>>>>>>>>>             >> Yasumasa
>>>>>>>>>>>             >>
>>>>>>>>>>>
>>>>>>>>>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
Hi David,

> You may have time to fix these in place before anyone else sees the webrev

I've fixed that:

  http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/


I checked raw text of your email, so I believe the indent is correct.
If I have mistake(s), please tell me.


Thanks,

Yasumasa


2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:

> Hi Yasumasa,
>
> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
>>
>> Hi all,
>>
>> This change was tried to push via JPRT, but it failed because
>> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
>> was failed.
>> Also I heard the comment that the changes in arguments.cpp which set
>> ergonomic value to CompressedClassSpaceSize should be moved to
>> Metaspace::ergo_initialize().
>>
>> I uploaded new webrev. Could you review again?
>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
>> test/hotspot/jtreg/runtime/SharedArchiveFile.
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
>
>
> I'll let Coleen cover the technical details, I'll just pick up a few style
> nits:
>
> 3328   /* Initial virtual space size will be calculated at
> global_initialize() */
>
> Use // comment
>
> 3329   size_t min_metaspace_sz =
> 3330                    VIRTUALSPACEMULTIPLIER *
> InitialBootClassLoaderMetaspaceSize;
>
>            ^ should indent only to here
>
> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
> 3337                                       MaxMetaspaceSize -
> min_metaspace_sz);
>                            ^ should indent only to here
>
> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
> 3342 min_metaspace_sz);
>
>                         ^ should indent only to here
>
> You may have time to fix these in place before anyone else sees the webrev
> :)
>
> Thanks,
> David
> -----
>
>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>>>
>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only
>>> needs
>>> one reviewer, unless there were more?  Please send me the result of hg
>>> export.
>>> thanks,
>>> Coleen
>>>
>>>
>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>>
>>>>
>>>> Hi Coleen,
>>>>
>>>>>> I will sponsor this for you.
>>>>
>>>>
>>>> Thanks!
>>>>
>>>>
>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>> check_metaspace_sizes()
>>>>>> so that you don't have to tell arguments what the MediumChunk size is.
>>>>
>>>>
>>>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>>>> to get minimum metaspace size, and uses return value from this
>>>> function in metaspace initialization and metaspace size check.
>>>>
>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>>
>>>> Could you review again?
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>>
>>>>>
>>>>> Thanks for you both updating and sponsoring the webrev!
>>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>>
>>>>> -Man
>>>>>
>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>>>>>
>>>>>>
>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>> check_metaspace_sizes()
>>>>>> so that you don't have to tell arguments what the MediumChunk size is.
>>>>>>
>>>>>> I will sponsor this for you.
>>>>>>
>>>>>> thanks,
>>>>>> Coleen
>>>>>>
>>>>>>
>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I added a testcase for this issue in new webrev:
>>>>>>>
>>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>>> Could you review it?
>>>>>>>>
>>>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>>>
>>>>>>>>
>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>>
>>>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>>>> internal
>>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>>> Is it possible to push it to P4?
>>>>>>>>>
>>>>>>>>> -Man
>>>>>>>>>
>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>>> <[hidden email]>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>>
>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>>>>> space
>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>> this
>>>>>>>>>>> on the
>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs after
>>>>>>>>>> repos
>>>>>>>>>> are opened.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi Man,
>>>>>>>>>>>
>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>>
>>>>>>>>>>>> Do you have any thoughts on why this patch has been pending for
>>>>>>>>>>>> 2+
>>>>>>>>>>>> years? This patch could really save us from some annoying issues
>>>>>>>>>>>> since we
>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>>>>> space
>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>> this
>>>>>>>>>>> on the
>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>
>>>>>>>>>>> StefanK
>>>>>>>>>>>
>>>>>>>>>>>> -Man
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>        Hi all,
>>>>>>>>>>>>
>>>>>>>>>>>>        I wonder if there is any recent update on the patch for
>>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>>        Is it possible to push this patch into JDK9? Except for
>>>>>>>>>>>> its
>>>>>>>>>>>> low
>>>>>>>>>>>>        priority (P5),
>>>>>>>>>>>>        is there any complication that prevents this patch
>>>>>>>>>>>> getting
>>>>>>>>>>>> approved
>>>>>>>>>>>>        (for example, some JVM logic requires
>>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>>> to be
>>>>>>>>>>>>        1GB by default)?
>>>>>>>>>>>>
>>>>>>>>>>>>        I work in the Java Platform Team at Google. We have
>>>>>>>>>>>> encountered
>>>>>>>>>>>>        annoying issues that the hsperfdata counter
>>>>>>>>>>>>        "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>>        a too large value (about 1GB) even if user sets
>>>>>>>>>>>>        -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>>        memory reserved by metaspace,
>>>>>>>>>>>>        regardless of MaxMetaspaceSize value. The root cause for
>>>>>>>>>>>> these
>>>>>>>>>>>>        issues is that CompressedClassSpaceSize is not
>>>>>>>>>>>> automatically
>>>>>>>>>>>> capped
>>>>>>>>>>>>        by MaxMetaspaceSize
>>>>>>>>>>>>        during VM initialization, and this patch seems fix the
>>>>>>>>>>>> root
>>>>>>>>>>>> cause.
>>>>>>>>>>>>        (I'm aware that even after this patch, the reserved size
>>>>>>>>>>>> could
>>>>>>>>>>>> still
>>>>>>>>>>>>        be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>>        but it is better than the current situation.)
>>>>>>>>>>>>
>>>>>>>>>>>>        Thanks,
>>>>>>>>>>>>        Man
>>>>>>>>>>>>
>>>>>>>>>>>>        On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>            Thank you for your comment!
>>>>>>>>>>>>             > Try running a debug JVM with your patch with this
>>>>>>>>>>>> command
>>>>>>>>>>>> line.
>>>>>>>>>>>>             >
>>>>>>>>>>>>             > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>            Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>>
>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>>            It works on fastdebug VM.
>>>>>>>>>>>>            Please review again.
>>>>>>>>>>>>
>>>>>>>>>>>>            Thanks,
>>>>>>>>>>>>            Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>            On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>>             > Yasumasa,
>>>>>>>>>>>>             >
>>>>>>>>>>>>             > Try running a debug JVM with your patch with this
>>>>>>>>>>>> command
>>>>>>>>>>>> line.
>>>>>>>>>>>>             >
>>>>>>>>>>>>             > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>             >
>>>>>>>>>>>>             > On a linux system I get this when I build with
>>>>>>>>>>>> your
>>>>>>>>>>>> patch.
>>>>>>>>>>>>             >
>>>>>>>>>>>>             >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>             >> # To suppress the following error report, specify
>>>>>>>>>>>> this
>>>>>>>>>>>> argument
>>>>>>>>>>>>             >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>>            SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>>             >> #
>>>>>>>>>>>>             >> # A fatal error has been detected by the Java
>>>>>>>>>>>> Runtime
>>>>>>>>>>>>            Environment:
>>>>>>>>>>>>             >> #
>>>>>>>>>>>>             >> #  Internal Error
>>>>>>>>>>>>             >>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>>             >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>>             >> #  assert(size > MediumChunk || size >
>>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>>            failed: Not a
>>>>>>>>>>>>             >> humongous chunk
>>>>>>>>>>>>             >> #
>>>>>>>>>>>>             >
>>>>>>>>>>>>             >
>>>>>>>>>>>>             > Jon
>>>>>>>>>>>>             >
>>>>>>>>>>>>             >
>>>>>>>>>>>>             > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>             >> I want to continue to discuss about
>>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>>            MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>>             >>
>>>>>>>>>>>>             >>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>>             >>>> Should I resize CompressedClassSpaceSize than
>>>>>>>>>>>> to
>>>>>>>>>>>> show
>>>>>>>>>>>>            error message?
>>>>>>>>>>>>             >>> If you add slightly better heuristics for the
>>>>>>>>>>>> setup
>>>>>>>>>>>> of
>>>>>>>>>>>> the
>>>>>>>>>>>>            CompressedClassSpaceSize flag, for example lowering
>>>>>>>>>>>> the
>>>>>>>>>>>>            CompressedClassSpaceSize when MaxMetaspaceSize is
>>>>>>>>>>>> set,
>>>>>>>>>>>> then
>>>>>>>>>>>> it
>>>>>>>>>>>>            might be less likely that you'll hit the
>>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>>> when
>>>>>>>>>>>>            the system is set up with strict overcommit settings.
>>>>>>>>>>>>             >>
>>>>>>>>>>>>             >> I've uploaded new webrev:
>>>>>>>>>>>>             >>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>>
>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>>             >>
>>>>>>>>>>>>             >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>>            CompressedClassSpaceSize, and
>>>>>>>>>>>>             >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>>             >>
>>>>>>>>>>>>             >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>>            Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>>             >> If InitialBootClassLoaderMetaspaceSize is greater
>>>>>>>>>>>> than
>>>>>>>>>>>>            MaxMetaspaceSize,
>>>>>>>>>>>>             >> VM will fail with error message.
>>>>>>>>>>>>             >>
>>>>>>>>>>>>             >> InitialBootClassLoaderMetaspaceSize will be set
>>>>>>>>>>>> to
>>>>>>>>>>>>            MaxMetaspaceSize
>>>>>>>>>>>>             >> when UseCompressedClassPointers is not set in
>>>>>>>>>>>>            Metaspace::ergo_initialize().
>>>>>>>>>>>>             >>
>>>>>>>>>>>>             >>
>>>>>>>>>>>>             >> Thanks,
>>>>>>>>>>>>             >>
>>>>>>>>>>>>             >> Yasumasa
>>>>>>>>>>>>             >>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

David Holmes
On 12/10/2017 2:47 PM, Yasumasa Suenaga wrote:

> Hi David,
>
>> You may have time to fix these in place before anyone else sees the webrev
>
> I've fixed that:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/
>
>
> I checked raw text of your email, so I believe the indent is correct.
> If I have mistake(s), please tell me.

Sorry they are off. Basic rules:

var =
     some long value;

indent by 4.

foo(arg1, arg2,
     arg3, arg4);

align the args on the two lines as shown.

Hope that is clearer.

Thanks,
David

>
> Thanks,
>
> Yasumasa
>
>
> 2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:
>> Hi Yasumasa,
>>
>> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
>>>
>>> Hi all,
>>>
>>> This change was tried to push via JPRT, but it failed because
>>> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
>>> was failed.
>>> Also I heard the comment that the changes in arguments.cpp which set
>>> ergonomic value to CompressedClassSpaceSize should be moved to
>>> Metaspace::ergo_initialize().
>>>
>>> I uploaded new webrev. Could you review again?
>>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
>>> test/hotspot/jtreg/runtime/SharedArchiveFile.
>>>
>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
>>
>>
>> I'll let Coleen cover the technical details, I'll just pick up a few style
>> nits:
>>
>> 3328   /* Initial virtual space size will be calculated at
>> global_initialize() */
>>
>> Use // comment
>>
>> 3329   size_t min_metaspace_sz =
>> 3330                    VIRTUALSPACEMULTIPLIER *
>> InitialBootClassLoaderMetaspaceSize;
>>
>>             ^ should indent only to here
>>
>> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
>> 3337                                       MaxMetaspaceSize -
>> min_metaspace_sz);
>>                             ^ should indent only to here
>>
>> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
>> 3342 min_metaspace_sz);
>>
>>                          ^ should indent only to here
>>
>> You may have time to fix these in place before anyone else sees the webrev
>> :)
>>
>> Thanks,
>> David
>> -----
>>
>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>
>>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>>>>
>>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only
>>>> needs
>>>> one reviewer, unless there were more?  Please send me the result of hg
>>>> export.
>>>> thanks,
>>>> Coleen
>>>>
>>>>
>>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>>>
>>>>>
>>>>> Hi Coleen,
>>>>>
>>>>>>> I will sponsor this for you.
>>>>>
>>>>>
>>>>> Thanks!
>>>>>
>>>>>
>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>> check_metaspace_sizes()
>>>>>>> so that you don't have to tell arguments what the MediumChunk size is.
>>>>>
>>>>>
>>>>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>>>>> to get minimum metaspace size, and uses return value from this
>>>>> function in metaspace initialization and metaspace size check.
>>>>>
>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>>>
>>>>> Could you review again?
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>
>>>>>>
>>>>>> Thanks for you both updating and sponsoring the webrev!
>>>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>>>
>>>>>> -Man
>>>>>>
>>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>> check_metaspace_sizes()
>>>>>>> so that you don't have to tell arguments what the MediumChunk size is.
>>>>>>>
>>>>>>> I will sponsor this for you.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Coleen
>>>>>>>
>>>>>>>
>>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> I added a testcase for this issue in new webrev:
>>>>>>>>
>>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>>>> Could you review it?
>>>>>>>>>
>>>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>>>
>>>>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>>>>> internal
>>>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>>>> Is it possible to push it to P4?
>>>>>>>>>>
>>>>>>>>>> -Man
>>>>>>>>>>
>>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>>>> <[hidden email]>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>>>
>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>>>>>> space
>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>> this
>>>>>>>>>>>> on the
>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs after
>>>>>>>>>>> repos
>>>>>>>>>>> are opened.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Man,
>>>>>>>>>>>>
>>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Do you have any thoughts on why this patch has been pending for
>>>>>>>>>>>>> 2+
>>>>>>>>>>>>> years? This patch could really save us from some annoying issues
>>>>>>>>>>>>> since we
>>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed class
>>>>>>>>>>>> space
>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>> this
>>>>>>>>>>>> on the
>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>
>>>>>>>>>>>> StefanK
>>>>>>>>>>>>
>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>         Hi all,
>>>>>>>>>>>>>
>>>>>>>>>>>>>         I wonder if there is any recent update on the patch for
>>>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>>>         Is it possible to push this patch into JDK9? Except for
>>>>>>>>>>>>> its
>>>>>>>>>>>>> low
>>>>>>>>>>>>>         priority (P5),
>>>>>>>>>>>>>         is there any complication that prevents this patch
>>>>>>>>>>>>> getting
>>>>>>>>>>>>> approved
>>>>>>>>>>>>>         (for example, some JVM logic requires
>>>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>>>> to be
>>>>>>>>>>>>>         1GB by default)?
>>>>>>>>>>>>>
>>>>>>>>>>>>>         I work in the Java Platform Team at Google. We have
>>>>>>>>>>>>> encountered
>>>>>>>>>>>>>         annoying issues that the hsperfdata counter
>>>>>>>>>>>>>         "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>>>         a too large value (about 1GB) even if user sets
>>>>>>>>>>>>>         -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>>>         memory reserved by metaspace,
>>>>>>>>>>>>>         regardless of MaxMetaspaceSize value. The root cause for
>>>>>>>>>>>>> these
>>>>>>>>>>>>>         issues is that CompressedClassSpaceSize is not
>>>>>>>>>>>>> automatically
>>>>>>>>>>>>> capped
>>>>>>>>>>>>>         by MaxMetaspaceSize
>>>>>>>>>>>>>         during VM initialization, and this patch seems fix the
>>>>>>>>>>>>> root
>>>>>>>>>>>>> cause.
>>>>>>>>>>>>>         (I'm aware that even after this patch, the reserved size
>>>>>>>>>>>>> could
>>>>>>>>>>>>> still
>>>>>>>>>>>>>         be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>>>         but it is better than the current situation.)
>>>>>>>>>>>>>
>>>>>>>>>>>>>         Thanks,
>>>>>>>>>>>>>         Man
>>>>>>>>>>>>>
>>>>>>>>>>>>>         On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>             Thank you for your comment!
>>>>>>>>>>>>>              > Try running a debug JVM with your patch with this
>>>>>>>>>>>>> command
>>>>>>>>>>>>> line.
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>             Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>>>
>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>>>             It works on fastdebug VM.
>>>>>>>>>>>>>             Please review again.
>>>>>>>>>>>>>
>>>>>>>>>>>>>             Thanks,
>>>>>>>>>>>>>             Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>             On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>>>              > Yasumasa,
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              > Try running a debug JVM with your patch with this
>>>>>>>>>>>>> command
>>>>>>>>>>>>> line.
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              > On a linux system I get this when I build with
>>>>>>>>>>>>> your
>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>              >> # To suppress the following error report, specify
>>>>>>>>>>>>> this
>>>>>>>>>>>>> argument
>>>>>>>>>>>>>              >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>>>             SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>>>              >> #
>>>>>>>>>>>>>              >> # A fatal error has been detected by the Java
>>>>>>>>>>>>> Runtime
>>>>>>>>>>>>>             Environment:
>>>>>>>>>>>>>              >> #
>>>>>>>>>>>>>              >> #  Internal Error
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>>>              >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>>>              >> #  assert(size > MediumChunk || size >
>>>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>>>             failed: Not a
>>>>>>>>>>>>>              >> humongous chunk
>>>>>>>>>>>>>              >> #
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              > Jon
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              >
>>>>>>>>>>>>>              > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>              >> I want to continue to discuss about
>>>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>>>             MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>>>              >>>> Should I resize CompressedClassSpaceSize than
>>>>>>>>>>>>> to
>>>>>>>>>>>>> show
>>>>>>>>>>>>>             error message?
>>>>>>>>>>>>>              >>> If you add slightly better heuristics for the
>>>>>>>>>>>>> setup
>>>>>>>>>>>>> of
>>>>>>>>>>>>> the
>>>>>>>>>>>>>             CompressedClassSpaceSize flag, for example lowering
>>>>>>>>>>>>> the
>>>>>>>>>>>>>             CompressedClassSpaceSize when MaxMetaspaceSize is
>>>>>>>>>>>>> set,
>>>>>>>>>>>>> then
>>>>>>>>>>>>> it
>>>>>>>>>>>>>             might be less likely that you'll hit the
>>>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>>>> when
>>>>>>>>>>>>>             the system is set up with strict overcommit settings.
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>              >> I've uploaded new webrev:
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>>>
>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>              >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>>>             CompressedClassSpaceSize, and
>>>>>>>>>>>>>              >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>              >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>>>             Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>>>              >> If InitialBootClassLoaderMetaspaceSize is greater
>>>>>>>>>>>>> than
>>>>>>>>>>>>>             MaxMetaspaceSize,
>>>>>>>>>>>>>              >> VM will fail with error message.
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>              >> InitialBootClassLoaderMetaspaceSize will be set
>>>>>>>>>>>>> to
>>>>>>>>>>>>>             MaxMetaspaceSize
>>>>>>>>>>>>>              >> when UseCompressedClassPointers is not set in
>>>>>>>>>>>>>             Metaspace::ergo_initialize().
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>              >> Thanks,
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>              >> Yasumasa
>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
Thanks David!

I've fixed:

  http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.08/


Yasumasa


2017-10-12 13:55 GMT+09:00 David Holmes <[hidden email]>:

> On 12/10/2017 2:47 PM, Yasumasa Suenaga wrote:
>>
>> Hi David,
>>
>>> You may have time to fix these in place before anyone else sees the
>>> webrev
>>
>>
>> I've fixed that:
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/
>>
>>
>> I checked raw text of your email, so I believe the indent is correct.
>> If I have mistake(s), please tell me.
>
>
> Sorry they are off. Basic rules:
>
> var =
>     some long value;
>
> indent by 4.
>
> foo(arg1, arg2,
>     arg3, arg4);
>
> align the args on the two lines as shown.
>
> Hope that is clearer.
>
> Thanks,
> David
>
>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> 2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:
>>>
>>> Hi Yasumasa,
>>>
>>> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
>>>>
>>>>
>>>> Hi all,
>>>>
>>>> This change was tried to push via JPRT, but it failed because
>>>> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
>>>> was failed.
>>>> Also I heard the comment that the changes in arguments.cpp which set
>>>> ergonomic value to CompressedClassSpaceSize should be moved to
>>>> Metaspace::ergo_initialize().
>>>>
>>>> I uploaded new webrev. Could you review again?
>>>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
>>>> test/hotspot/jtreg/runtime/SharedArchiveFile.
>>>>
>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
>>>
>>>
>>>
>>> I'll let Coleen cover the technical details, I'll just pick up a few
>>> style
>>> nits:
>>>
>>> 3328   /* Initial virtual space size will be calculated at
>>> global_initialize() */
>>>
>>> Use // comment
>>>
>>> 3329   size_t min_metaspace_sz =
>>> 3330                    VIRTUALSPACEMULTIPLIER *
>>> InitialBootClassLoaderMetaspaceSize;
>>>
>>>             ^ should indent only to here
>>>
>>> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
>>> 3337                                       MaxMetaspaceSize -
>>> min_metaspace_sz);
>>>                             ^ should indent only to here
>>>
>>> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
>>> 3342 min_metaspace_sz);
>>>
>>>                          ^ should indent only to here
>>>
>>> You may have time to fix these in place before anyone else sees the
>>> webrev
>>> :)
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>
>>>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>>>>>
>>>>>
>>>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only
>>>>> needs
>>>>> one reviewer, unless there were more?  Please send me the result of hg
>>>>> export.
>>>>> thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Coleen,
>>>>>>
>>>>>>>> I will sponsor this for you.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>>
>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>> check_metaspace_sizes()
>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>> is.
>>>>>>
>>>>>>
>>>>>>
>>>>>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>>>>>> to get minimum metaspace size, and uses return value from this
>>>>>> function in metaspace initialization and metaspace size check.
>>>>>>
>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>>>>
>>>>>> Could you review again?
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks for you both updating and sponsoring the webrev!
>>>>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>>>>
>>>>>>> -Man
>>>>>>>
>>>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>> check_metaspace_sizes()
>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>> is.
>>>>>>>>
>>>>>>>> I will sponsor this for you.
>>>>>>>>
>>>>>>>> thanks,
>>>>>>>> Coleen
>>>>>>>>
>>>>>>>>
>>>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> I added a testcase for this issue in new webrev:
>>>>>>>>>
>>>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>>>>> Could you review it?
>>>>>>>>>>
>>>>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>>>>
>>>>>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>>>>>> internal
>>>>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>>>>> Is it possible to push it to P4?
>>>>>>>>>>>
>>>>>>>>>>> -Man
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>>>>> <[hidden email]>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>>>>
>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>> class
>>>>>>>>>>>>> space
>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>>> this
>>>>>>>>>>>>> on the
>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs
>>>>>>>>>>>> after
>>>>>>>>>>>> repos
>>>>>>>>>>>> are opened.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Man,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Do you have any thoughts on why this patch has been pending
>>>>>>>>>>>>>> for
>>>>>>>>>>>>>> 2+
>>>>>>>>>>>>>> years? This patch could really save us from some annoying
>>>>>>>>>>>>>> issues
>>>>>>>>>>>>>> since we
>>>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>> class
>>>>>>>>>>>>> space
>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>>> this
>>>>>>>>>>>>> on the
>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>
>>>>>>>>>>>>> StefanK
>>>>>>>>>>>>>
>>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         Hi all,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         I wonder if there is any recent update on the patch
>>>>>>>>>>>>>> for
>>>>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>>>>         Is it possible to push this patch into JDK9? Except
>>>>>>>>>>>>>> for
>>>>>>>>>>>>>> its
>>>>>>>>>>>>>> low
>>>>>>>>>>>>>>         priority (P5),
>>>>>>>>>>>>>>         is there any complication that prevents this patch
>>>>>>>>>>>>>> getting
>>>>>>>>>>>>>> approved
>>>>>>>>>>>>>>         (for example, some JVM logic requires
>>>>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>         1GB by default)?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         I work in the Java Platform Team at Google. We have
>>>>>>>>>>>>>> encountered
>>>>>>>>>>>>>>         annoying issues that the hsperfdata counter
>>>>>>>>>>>>>>         "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>>>>         a too large value (about 1GB) even if user sets
>>>>>>>>>>>>>>         -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>>>>         memory reserved by metaspace,
>>>>>>>>>>>>>>         regardless of MaxMetaspaceSize value. The root cause
>>>>>>>>>>>>>> for
>>>>>>>>>>>>>> these
>>>>>>>>>>>>>>         issues is that CompressedClassSpaceSize is not
>>>>>>>>>>>>>> automatically
>>>>>>>>>>>>>> capped
>>>>>>>>>>>>>>         by MaxMetaspaceSize
>>>>>>>>>>>>>>         during VM initialization, and this patch seems fix the
>>>>>>>>>>>>>> root
>>>>>>>>>>>>>> cause.
>>>>>>>>>>>>>>         (I'm aware that even after this patch, the reserved
>>>>>>>>>>>>>> size
>>>>>>>>>>>>>> could
>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>         be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>>>>         but it is better than the current situation.)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         Thanks,
>>>>>>>>>>>>>>         Man
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>             Thank you for your comment!
>>>>>>>>>>>>>>              > Try running a debug JVM with your patch with
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> command
>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>             Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>>>>             It works on fastdebug VM.
>>>>>>>>>>>>>>             Please review again.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>             Thanks,
>>>>>>>>>>>>>>             Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>             On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>>>>              > Yasumasa,
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              > Try running a debug JVM with your patch with
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> command
>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              > On a linux system I get this when I build with
>>>>>>>>>>>>>> your
>>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>              >> # To suppress the following error report,
>>>>>>>>>>>>>> specify
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> argument
>>>>>>>>>>>>>>              >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>>>>             SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>>>>              >> #
>>>>>>>>>>>>>>              >> # A fatal error has been detected by the Java
>>>>>>>>>>>>>> Runtime
>>>>>>>>>>>>>>             Environment:
>>>>>>>>>>>>>>              >> #
>>>>>>>>>>>>>>              >> #  Internal Error
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>>>>              >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>>>>              >> #  assert(size > MediumChunk || size >
>>>>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>>>>             failed: Not a
>>>>>>>>>>>>>>              >> humongous chunk
>>>>>>>>>>>>>>              >> #
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              > Jon
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              >
>>>>>>>>>>>>>>              > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>              >> I want to continue to discuss about
>>>>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>>>>             MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>>>>              >>>> Should I resize CompressedClassSpaceSize
>>>>>>>>>>>>>> than
>>>>>>>>>>>>>> to
>>>>>>>>>>>>>> show
>>>>>>>>>>>>>>             error message?
>>>>>>>>>>>>>>              >>> If you add slightly better heuristics for the
>>>>>>>>>>>>>> setup
>>>>>>>>>>>>>> of
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>             CompressedClassSpaceSize flag, for example
>>>>>>>>>>>>>> lowering
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>             CompressedClassSpaceSize when MaxMetaspaceSize is
>>>>>>>>>>>>>> set,
>>>>>>>>>>>>>> then
>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>             might be less likely that you'll hit the
>>>>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>>>>> when
>>>>>>>>>>>>>>             the system is set up with strict overcommit
>>>>>>>>>>>>>> settings.
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>              >> I've uploaded new webrev:
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>              >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>>>>             CompressedClassSpaceSize, and
>>>>>>>>>>>>>>              >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>              >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>>>>             Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>>>>              >> If InitialBootClassLoaderMetaspaceSize is
>>>>>>>>>>>>>> greater
>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>             MaxMetaspaceSize,
>>>>>>>>>>>>>>              >> VM will fail with error message.
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>              >> InitialBootClassLoaderMetaspaceSize will be
>>>>>>>>>>>>>> set
>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>             MaxMetaspaceSize
>>>>>>>>>>>>>>              >> when UseCompressedClassPointers is not set in
>>>>>>>>>>>>>>             Metaspace::ergo_initialize().
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>              >> Thanks,
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>              >> Yasumasa
>>>>>>>>>>>>>>              >>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

David Holmes
Thanks.

David

On 12/10/2017 3:16 PM, Yasumasa Suenaga wrote:

> Thanks David!
>
> I've fixed:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.08/
>
>
> Yasumasa
>
>
> 2017-10-12 13:55 GMT+09:00 David Holmes <[hidden email]>:
>> On 12/10/2017 2:47 PM, Yasumasa Suenaga wrote:
>>>
>>> Hi David,
>>>
>>>> You may have time to fix these in place before anyone else sees the
>>>> webrev
>>>
>>>
>>> I've fixed that:
>>>
>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/
>>>
>>>
>>> I checked raw text of your email, so I believe the indent is correct.
>>> If I have mistake(s), please tell me.
>>
>>
>> Sorry they are off. Basic rules:
>>
>> var =
>>      some long value;
>>
>> indent by 4.
>>
>> foo(arg1, arg2,
>>      arg3, arg4);
>>
>> align the args on the two lines as shown.
>>
>> Hope that is clearer.
>>
>> Thanks,
>> David
>>
>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> 2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:
>>>>
>>>> Hi Yasumasa,
>>>>
>>>> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
>>>>>
>>>>>
>>>>> Hi all,
>>>>>
>>>>> This change was tried to push via JPRT, but it failed because
>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
>>>>> was failed.
>>>>> Also I heard the comment that the changes in arguments.cpp which set
>>>>> ergonomic value to CompressedClassSpaceSize should be moved to
>>>>> Metaspace::ergo_initialize().
>>>>>
>>>>> I uploaded new webrev. Could you review again?
>>>>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile.
>>>>>
>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
>>>>
>>>>
>>>>
>>>> I'll let Coleen cover the technical details, I'll just pick up a few
>>>> style
>>>> nits:
>>>>
>>>> 3328   /* Initial virtual space size will be calculated at
>>>> global_initialize() */
>>>>
>>>> Use // comment
>>>>
>>>> 3329   size_t min_metaspace_sz =
>>>> 3330                    VIRTUALSPACEMULTIPLIER *
>>>> InitialBootClassLoaderMetaspaceSize;
>>>>
>>>>              ^ should indent only to here
>>>>
>>>> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
>>>> 3337                                       MaxMetaspaceSize -
>>>> min_metaspace_sz);
>>>>                              ^ should indent only to here
>>>>
>>>> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
>>>> 3342 min_metaspace_sz);
>>>>
>>>>                           ^ should indent only to here
>>>>
>>>> You may have time to fix these in place before anyone else sees the
>>>> webrev
>>>> :)
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>
>>>>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>>>>>>
>>>>>>
>>>>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only
>>>>>> needs
>>>>>> one reviewer, unless there were more?  Please send me the result of hg
>>>>>> export.
>>>>>> thanks,
>>>>>> Coleen
>>>>>>
>>>>>>
>>>>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi Coleen,
>>>>>>>
>>>>>>>>> I will sponsor this for you.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>
>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>> check_metaspace_sizes()
>>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>>> is.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>>>>>>> to get minimum metaspace size, and uses return value from this
>>>>>>> function in metaspace initialization and metaspace size check.
>>>>>>>
>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>>>>>
>>>>>>> Could you review again?
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for you both updating and sponsoring the webrev!
>>>>>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>>>>>
>>>>>>>> -Man
>>>>>>>>
>>>>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>> check_metaspace_sizes()
>>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>>> is.
>>>>>>>>>
>>>>>>>>> I will sponsor this for you.
>>>>>>>>>
>>>>>>>>> thanks,
>>>>>>>>> Coleen
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> I added a testcase for this issue in new webrev:
>>>>>>>>>>
>>>>>>>>>>        http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>>>>>> Could you review it?
>>>>>>>>>>>
>>>>>>>>>>>        http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>>>>>
>>>>>>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>>>>>>> internal
>>>>>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>>>>>> Is it possible to push it to P4?
>>>>>>>>>>>>
>>>>>>>>>>>> -Man
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>>>>>> <[hidden email]>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>> class
>>>>>>>>>>>>>> space
>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs
>>>>>>>>>>>>> after
>>>>>>>>>>>>> repos
>>>>>>>>>>>>> are opened.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Man,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Do you have any thoughts on why this patch has been pending
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> 2+
>>>>>>>>>>>>>>> years? This patch could really save us from some annoying
>>>>>>>>>>>>>>> issues
>>>>>>>>>>>>>>> since we
>>>>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>> class
>>>>>>>>>>>>>> space
>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> StefanK
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          Hi all,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          I wonder if there is any recent update on the patch
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>>>>>          Is it possible to push this patch into JDK9? Except
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> its
>>>>>>>>>>>>>>> low
>>>>>>>>>>>>>>>          priority (P5),
>>>>>>>>>>>>>>>          is there any complication that prevents this patch
>>>>>>>>>>>>>>> getting
>>>>>>>>>>>>>>> approved
>>>>>>>>>>>>>>>          (for example, some JVM logic requires
>>>>>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>          1GB by default)?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          I work in the Java Platform Team at Google. We have
>>>>>>>>>>>>>>> encountered
>>>>>>>>>>>>>>>          annoying issues that the hsperfdata counter
>>>>>>>>>>>>>>>          "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>>>>>          a too large value (about 1GB) even if user sets
>>>>>>>>>>>>>>>          -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>>>>>          memory reserved by metaspace,
>>>>>>>>>>>>>>>          regardless of MaxMetaspaceSize value. The root cause
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> these
>>>>>>>>>>>>>>>          issues is that CompressedClassSpaceSize is not
>>>>>>>>>>>>>>> automatically
>>>>>>>>>>>>>>> capped
>>>>>>>>>>>>>>>          by MaxMetaspaceSize
>>>>>>>>>>>>>>>          during VM initialization, and this patch seems fix the
>>>>>>>>>>>>>>> root
>>>>>>>>>>>>>>> cause.
>>>>>>>>>>>>>>>          (I'm aware that even after this patch, the reserved
>>>>>>>>>>>>>>> size
>>>>>>>>>>>>>>> could
>>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>>          be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>>>>>          but it is better than the current situation.)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          Thanks,
>>>>>>>>>>>>>>>          Man
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              Thank you for your comment!
>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch with
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>              Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>>>>>              It works on fastdebug VM.
>>>>>>>>>>>>>>>              Please review again.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              Thanks,
>>>>>>>>>>>>>>>              Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>>>>>               > Yasumasa,
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch with
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > On a linux system I get this when I build with
>>>>>>>>>>>>>>> your
>>>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>               >> # To suppress the following error report,
>>>>>>>>>>>>>>> specify
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> argument
>>>>>>>>>>>>>>>               >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>>>>>              SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >> # A fatal error has been detected by the Java
>>>>>>>>>>>>>>> Runtime
>>>>>>>>>>>>>>>              Environment:
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >> #  Internal Error
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>>>>>               >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>>>>>               >> #  assert(size > MediumChunk || size >
>>>>>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>>>>>              failed: Not a
>>>>>>>>>>>>>>>               >> humongous chunk
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > Jon
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>               >> I want to continue to discuss about
>>>>>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>>>>>              MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>>>>>               >>>> Should I resize CompressedClassSpaceSize
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>> show
>>>>>>>>>>>>>>>              error message?
>>>>>>>>>>>>>>>               >>> If you add slightly better heuristics for the
>>>>>>>>>>>>>>> setup
>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>              CompressedClassSpaceSize flag, for example
>>>>>>>>>>>>>>> lowering
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>              CompressedClassSpaceSize when MaxMetaspaceSize is
>>>>>>>>>>>>>>> set,
>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>              might be less likely that you'll hit the
>>>>>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>>>>>> when
>>>>>>>>>>>>>>>              the system is set up with strict overcommit
>>>>>>>>>>>>>>> settings.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> I've uploaded new webrev:
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>>>>>              CompressedClassSpaceSize, and
>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>>>>>              Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>>>>>               >> If InitialBootClassLoaderMetaspaceSize is
>>>>>>>>>>>>>>> greater
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>              MaxMetaspaceSize,
>>>>>>>>>>>>>>>               >> VM will fail with error message.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize will be
>>>>>>>>>>>>>>> set
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>              MaxMetaspaceSize
>>>>>>>>>>>>>>>               >> when UseCompressedClassPointers is not set in
>>>>>>>>>>>>>>>              Metaspace::ergo_initialize().
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> Thanks,
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> Yasumasa
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>
>>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

harold seigel
In reply to this post by Yasumasa Suenaga-4
This looks good.

Thanks, Harold


On 10/12/2017 1:16 AM, Yasumasa Suenaga wrote:

> Thanks David!
>
> I've fixed:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.08/
>
>
> Yasumasa
>
>
> 2017-10-12 13:55 GMT+09:00 David Holmes <[hidden email]>:
>> On 12/10/2017 2:47 PM, Yasumasa Suenaga wrote:
>>> Hi David,
>>>
>>>> You may have time to fix these in place before anyone else sees the
>>>> webrev
>>>
>>> I've fixed that:
>>>
>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/
>>>
>>>
>>> I checked raw text of your email, so I believe the indent is correct.
>>> If I have mistake(s), please tell me.
>>
>> Sorry they are off. Basic rules:
>>
>> var =
>>      some long value;
>>
>> indent by 4.
>>
>> foo(arg1, arg2,
>>      arg3, arg4);
>>
>> align the args on the two lines as shown.
>>
>> Hope that is clearer.
>>
>> Thanks,
>> David
>>
>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> 2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:
>>>> Hi Yasumasa,
>>>>
>>>> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> This change was tried to push via JPRT, but it failed because
>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
>>>>> was failed.
>>>>> Also I heard the comment that the changes in arguments.cpp which set
>>>>> ergonomic value to CompressedClassSpaceSize should be moved to
>>>>> Metaspace::ergo_initialize().
>>>>>
>>>>> I uploaded new webrev. Could you review again?
>>>>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile.
>>>>>
>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
>>>>
>>>>
>>>> I'll let Coleen cover the technical details, I'll just pick up a few
>>>> style
>>>> nits:
>>>>
>>>> 3328   /* Initial virtual space size will be calculated at
>>>> global_initialize() */
>>>>
>>>> Use // comment
>>>>
>>>> 3329   size_t min_metaspace_sz =
>>>> 3330                    VIRTUALSPACEMULTIPLIER *
>>>> InitialBootClassLoaderMetaspaceSize;
>>>>
>>>>              ^ should indent only to here
>>>>
>>>> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
>>>> 3337                                       MaxMetaspaceSize -
>>>> min_metaspace_sz);
>>>>                              ^ should indent only to here
>>>>
>>>> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
>>>> 3342 min_metaspace_sz);
>>>>
>>>>                           ^ should indent only to here
>>>>
>>>> You may have time to fix these in place before anyone else sees the
>>>> webrev
>>>> :)
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>
>>>>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>>>>>>
>>>>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only
>>>>>> needs
>>>>>> one reviewer, unless there were more?  Please send me the result of hg
>>>>>> export.
>>>>>> thanks,
>>>>>> Coleen
>>>>>>
>>>>>>
>>>>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi Coleen,
>>>>>>>
>>>>>>>>> I will sponsor this for you.
>>>>>>>
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>
>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>> check_metaspace_sizes()
>>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>>> is.
>>>>>>>
>>>>>>>
>>>>>>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>>>>>>> to get minimum metaspace size, and uses return value from this
>>>>>>> function in metaspace initialization and metaspace size check.
>>>>>>>
>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>>>>>
>>>>>>> Could you review again?
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for you both updating and sponsoring the webrev!
>>>>>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>>>>>
>>>>>>>> -Man
>>>>>>>>
>>>>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>> check_metaspace_sizes()
>>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>>> is.
>>>>>>>>>
>>>>>>>>> I will sponsor this for you.
>>>>>>>>>
>>>>>>>>> thanks,
>>>>>>>>> Coleen
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> I added a testcase for this issue in new webrev:
>>>>>>>>>>
>>>>>>>>>>        http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>>>>>> Could you review it?
>>>>>>>>>>>
>>>>>>>>>>>        http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>>>>>
>>>>>>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>>>>>>> internal
>>>>>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>>>>>> Is it possible to push it to P4?
>>>>>>>>>>>>
>>>>>>>>>>>> -Man
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>>>>>> <[hidden email]>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>> class
>>>>>>>>>>>>>> space
>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs
>>>>>>>>>>>>> after
>>>>>>>>>>>>> repos
>>>>>>>>>>>>> are opened.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Man,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Do you have any thoughts on why this patch has been pending
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> 2+
>>>>>>>>>>>>>>> years? This patch could really save us from some annoying
>>>>>>>>>>>>>>> issues
>>>>>>>>>>>>>>> since we
>>>>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>> class
>>>>>>>>>>>>>> space
>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> StefanK
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          Hi all,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          I wonder if there is any recent update on the patch
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>>>>>          Is it possible to push this patch into JDK9? Except
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> its
>>>>>>>>>>>>>>> low
>>>>>>>>>>>>>>>          priority (P5),
>>>>>>>>>>>>>>>          is there any complication that prevents this patch
>>>>>>>>>>>>>>> getting
>>>>>>>>>>>>>>> approved
>>>>>>>>>>>>>>>          (for example, some JVM logic requires
>>>>>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>          1GB by default)?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          I work in the Java Platform Team at Google. We have
>>>>>>>>>>>>>>> encountered
>>>>>>>>>>>>>>>          annoying issues that the hsperfdata counter
>>>>>>>>>>>>>>>          "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>>>>>          a too large value (about 1GB) even if user sets
>>>>>>>>>>>>>>>          -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>>>>>          memory reserved by metaspace,
>>>>>>>>>>>>>>>          regardless of MaxMetaspaceSize value. The root cause
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> these
>>>>>>>>>>>>>>>          issues is that CompressedClassSpaceSize is not
>>>>>>>>>>>>>>> automatically
>>>>>>>>>>>>>>> capped
>>>>>>>>>>>>>>>          by MaxMetaspaceSize
>>>>>>>>>>>>>>>          during VM initialization, and this patch seems fix the
>>>>>>>>>>>>>>> root
>>>>>>>>>>>>>>> cause.
>>>>>>>>>>>>>>>          (I'm aware that even after this patch, the reserved
>>>>>>>>>>>>>>> size
>>>>>>>>>>>>>>> could
>>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>>          be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>>>>>          but it is better than the current situation.)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          Thanks,
>>>>>>>>>>>>>>>          Man
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              Thank you for your comment!
>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch with
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>              Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>>>>>              It works on fastdebug VM.
>>>>>>>>>>>>>>>              Please review again.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              Thanks,
>>>>>>>>>>>>>>>              Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>>>>>               > Yasumasa,
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch with
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > On a linux system I get this when I build with
>>>>>>>>>>>>>>> your
>>>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>               >> # To suppress the following error report,
>>>>>>>>>>>>>>> specify
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> argument
>>>>>>>>>>>>>>>               >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>>>>>              SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >> # A fatal error has been detected by the Java
>>>>>>>>>>>>>>> Runtime
>>>>>>>>>>>>>>>              Environment:
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >> #  Internal Error
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>>>>>               >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>>>>>               >> #  assert(size > MediumChunk || size >
>>>>>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>>>>>              failed: Not a
>>>>>>>>>>>>>>>               >> humongous chunk
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > Jon
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>               >> I want to continue to discuss about
>>>>>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>>>>>              MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>>>>>               >>>> Should I resize CompressedClassSpaceSize
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>> show
>>>>>>>>>>>>>>>              error message?
>>>>>>>>>>>>>>>               >>> If you add slightly better heuristics for the
>>>>>>>>>>>>>>> setup
>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>              CompressedClassSpaceSize flag, for example
>>>>>>>>>>>>>>> lowering
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>              CompressedClassSpaceSize when MaxMetaspaceSize is
>>>>>>>>>>>>>>> set,
>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>              might be less likely that you'll hit the
>>>>>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>>>>>> when
>>>>>>>>>>>>>>>              the system is set up with strict overcommit
>>>>>>>>>>>>>>> settings.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> I've uploaded new webrev:
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>>>>>              CompressedClassSpaceSize, and
>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>>>>>              Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>>>>>               >> If InitialBootClassLoaderMetaspaceSize is
>>>>>>>>>>>>>>> greater
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>              MaxMetaspaceSize,
>>>>>>>>>>>>>>>               >> VM will fail with error message.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize will be
>>>>>>>>>>>>>>> set
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>              MaxMetaspaceSize
>>>>>>>>>>>>>>>               >> when UseCompressedClassPointers is not set in
>>>>>>>>>>>>>>>              Metaspace::ergo_initialize().
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> Thanks,
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> Yasumasa
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

coleen.phillimore
In reply to this post by Yasumasa Suenaga-4

Hi Yasumasa,
This looks good.  I can sponsor this for you.
thanks,
Coleen

On 10/12/17 1:16 AM, Yasumasa Suenaga wrote:

> Thanks David!
>
> I've fixed:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.08/
>
>
> Yasumasa
>
>
> 2017-10-12 13:55 GMT+09:00 David Holmes <[hidden email]>:
>> On 12/10/2017 2:47 PM, Yasumasa Suenaga wrote:
>>> Hi David,
>>>
>>>> You may have time to fix these in place before anyone else sees the
>>>> webrev
>>>
>>> I've fixed that:
>>>
>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/
>>>
>>>
>>> I checked raw text of your email, so I believe the indent is correct.
>>> If I have mistake(s), please tell me.
>>
>> Sorry they are off. Basic rules:
>>
>> var =
>>      some long value;
>>
>> indent by 4.
>>
>> foo(arg1, arg2,
>>      arg3, arg4);
>>
>> align the args on the two lines as shown.
>>
>> Hope that is clearer.
>>
>> Thanks,
>> David
>>
>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> 2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:
>>>> Hi Yasumasa,
>>>>
>>>> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> This change was tried to push via JPRT, but it failed because
>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
>>>>> was failed.
>>>>> Also I heard the comment that the changes in arguments.cpp which set
>>>>> ergonomic value to CompressedClassSpaceSize should be moved to
>>>>> Metaspace::ergo_initialize().
>>>>>
>>>>> I uploaded new webrev. Could you review again?
>>>>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile.
>>>>>
>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
>>>>
>>>>
>>>> I'll let Coleen cover the technical details, I'll just pick up a few
>>>> style
>>>> nits:
>>>>
>>>> 3328   /* Initial virtual space size will be calculated at
>>>> global_initialize() */
>>>>
>>>> Use // comment
>>>>
>>>> 3329   size_t min_metaspace_sz =
>>>> 3330                    VIRTUALSPACEMULTIPLIER *
>>>> InitialBootClassLoaderMetaspaceSize;
>>>>
>>>>              ^ should indent only to here
>>>>
>>>> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
>>>> 3337                                       MaxMetaspaceSize -
>>>> min_metaspace_sz);
>>>>                              ^ should indent only to here
>>>>
>>>> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
>>>> 3342 min_metaspace_sz);
>>>>
>>>>                           ^ should indent only to here
>>>>
>>>> You may have time to fix these in place before anyone else sees the
>>>> webrev
>>>> :)
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>
>>>>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>>>>>>
>>>>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it only
>>>>>> needs
>>>>>> one reviewer, unless there were more?  Please send me the result of hg
>>>>>> export.
>>>>>> thanks,
>>>>>> Coleen
>>>>>>
>>>>>>
>>>>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi Coleen,
>>>>>>>
>>>>>>>>> I will sponsor this for you.
>>>>>>>
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>
>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>> check_metaspace_sizes()
>>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>>> is.
>>>>>>>
>>>>>>>
>>>>>>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>>>>>>> to get minimum metaspace size, and uses return value from this
>>>>>>> function in metaspace initialization and metaspace size check.
>>>>>>>
>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>>>>>
>>>>>>> Could you review again?
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for you both updating and sponsoring the webrev!
>>>>>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>>>>>
>>>>>>>> -Man
>>>>>>>>
>>>>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>> check_metaspace_sizes()
>>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>>> is.
>>>>>>>>>
>>>>>>>>> I will sponsor this for you.
>>>>>>>>>
>>>>>>>>> thanks,
>>>>>>>>> Coleen
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> I added a testcase for this issue in new webrev:
>>>>>>>>>>
>>>>>>>>>>        http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]>:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>>>>>> Could you review it?
>>>>>>>>>>>
>>>>>>>>>>>        http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>>>>>
>>>>>>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>>>>>>> internal
>>>>>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>>>>>> Is it possible to push it to P4?
>>>>>>>>>>>>
>>>>>>>>>>>> -Man
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>>>>>> <[hidden email]>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>> class
>>>>>>>>>>>>>> space
>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs
>>>>>>>>>>>>> after
>>>>>>>>>>>>> repos
>>>>>>>>>>>>> are opened.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Man,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Do you have any thoughts on why this patch has been pending
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> 2+
>>>>>>>>>>>>>>> years? This patch could really save us from some annoying
>>>>>>>>>>>>>>> issues
>>>>>>>>>>>>>>> since we
>>>>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>> class
>>>>>>>>>>>>>> space
>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction for
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> StefanK
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          Hi all,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          I wonder if there is any recent update on the patch
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>>>>>          Is it possible to push this patch into JDK9? Except
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> its
>>>>>>>>>>>>>>> low
>>>>>>>>>>>>>>>          priority (P5),
>>>>>>>>>>>>>>>          is there any complication that prevents this patch
>>>>>>>>>>>>>>> getting
>>>>>>>>>>>>>>> approved
>>>>>>>>>>>>>>>          (for example, some JVM logic requires
>>>>>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>          1GB by default)?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          I work in the Java Platform Team at Google. We have
>>>>>>>>>>>>>>> encountered
>>>>>>>>>>>>>>>          annoying issues that the hsperfdata counter
>>>>>>>>>>>>>>>          "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>>>>>          a too large value (about 1GB) even if user sets
>>>>>>>>>>>>>>>          -XX:MaxMetaspaceSize=100m, as well as GC log shows the
>>>>>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>>>>>          memory reserved by metaspace,
>>>>>>>>>>>>>>>          regardless of MaxMetaspaceSize value. The root cause
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> these
>>>>>>>>>>>>>>>          issues is that CompressedClassSpaceSize is not
>>>>>>>>>>>>>>> automatically
>>>>>>>>>>>>>>> capped
>>>>>>>>>>>>>>>          by MaxMetaspaceSize
>>>>>>>>>>>>>>>          during VM initialization, and this patch seems fix the
>>>>>>>>>>>>>>> root
>>>>>>>>>>>>>>> cause.
>>>>>>>>>>>>>>>          (I'm aware that even after this patch, the reserved
>>>>>>>>>>>>>>> size
>>>>>>>>>>>>>>> could
>>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>>          be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>>>>>          but it is better than the current situation.)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          Thanks,
>>>>>>>>>>>>>>>          Man
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>          On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              Thank you for your comment!
>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch with
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>              Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>>>>>              It works on fastdebug VM.
>>>>>>>>>>>>>>>              Please review again.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              Thanks,
>>>>>>>>>>>>>>>              Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>              On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>>>>>               > Yasumasa,
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch with
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > On a linux system I get this when I build with
>>>>>>>>>>>>>>> your
>>>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>               >> # To suppress the following error report,
>>>>>>>>>>>>>>> specify
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> argument
>>>>>>>>>>>>>>>               >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>>>>>              SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >> # A fatal error has been detected by the Java
>>>>>>>>>>>>>>> Runtime
>>>>>>>>>>>>>>>              Environment:
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >> #  Internal Error
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>>>>>               >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>>>>>               >> #  assert(size > MediumChunk || size >
>>>>>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>>>>>              failed: Not a
>>>>>>>>>>>>>>>               >> humongous chunk
>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > Jon
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>               > On 6/17/2015 7:54 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>               >> I want to continue to discuss about
>>>>>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>>>>>              MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>>>>>               >>>> Should I resize CompressedClassSpaceSize
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>> show
>>>>>>>>>>>>>>>              error message?
>>>>>>>>>>>>>>>               >>> If you add slightly better heuristics for the
>>>>>>>>>>>>>>> setup
>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>              CompressedClassSpaceSize flag, for example
>>>>>>>>>>>>>>> lowering
>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>              CompressedClassSpaceSize when MaxMetaspaceSize is
>>>>>>>>>>>>>>> set,
>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>              might be less likely that you'll hit the
>>>>>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>>>>>> when
>>>>>>>>>>>>>>>              the system is set up with strict overcommit
>>>>>>>>>>>>>>> settings.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> I've uploaded new webrev:
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>>>>>              CompressedClassSpaceSize, and
>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>>>>>              Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>>>>>               >> If InitialBootClassLoaderMetaspaceSize is
>>>>>>>>>>>>>>> greater
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>              MaxMetaspaceSize,
>>>>>>>>>>>>>>>               >> VM will fail with error message.
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize will be
>>>>>>>>>>>>>>> set
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>              MaxMetaspaceSize
>>>>>>>>>>>>>>>               >> when UseCompressedClassPointers is not set in
>>>>>>>>>>>>>>>              Metaspace::ergo_initialize().
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> Thanks,
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>               >> Yasumasa
>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Man Cao
Hi Yasumasa,

I'm looking at if this patch can fix the original issue with hsperfdata
counters sun_gc_metaspace_maxCapacity and
sun_gc_compressedclassspace_maxCapacity being too large when user has set
-XX:MaxMetaspaceSize=100m or some value less than 1GB. However, it does
not, these counters still reports around 1GB if MaxMetaspaceSize is less
than 1GB.

The reason is that the code added in this patch to
Metaspace::ergo_initialize()  is AFTER the two lines:

 CompressedClassSpaceSize =
align_size_down_bounded(CompressedClassSpaceSize, _reserve_alignment);
 set_compressed_class_space_size(CompressedClassSpaceSize);

So set_compressed_class_space_size() still gets the old value
of CompressedClassSpaceSize that is too large, making the reserved size of
metaspace too large.

Should these two lines be moved after the code added in this patch?

Thanks,
-Man

On Tue, Oct 17, 2017 at 12:46 PM, <[hidden email]> wrote:

>
> Hi Yasumasa,
> This looks good.  I can sponsor this for you.
> thanks,
> Coleen
>
>
> On 10/12/17 1:16 AM, Yasumasa Suenaga wrote:
>
>> Thanks David!
>>
>> I've fixed:
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.08/
>>
>>
>> Yasumasa
>>
>>
>> 2017-10-12 13:55 GMT+09:00 David Holmes <[hidden email]>:
>>
>>> On 12/10/2017 2:47 PM, Yasumasa Suenaga wrote:
>>>
>>>> Hi David,
>>>>
>>>> You may have time to fix these in place before anyone else sees the
>>>>> webrev
>>>>>
>>>>
>>>> I've fixed that:
>>>>
>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/
>>>>
>>>>
>>>> I checked raw text of your email, so I believe the indent is correct.
>>>> If I have mistake(s), please tell me.
>>>>
>>>
>>> Sorry they are off. Basic rules:
>>>
>>> var =
>>>      some long value;
>>>
>>> indent by 4.
>>>
>>> foo(arg1, arg2,
>>>      arg3, arg4);
>>>
>>> align the args on the two lines as shown.
>>>
>>> Hope that is clearer.
>>>
>>> Thanks,
>>> David
>>>
>>>
>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> 2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:
>>>>
>>>>> Hi Yasumasa,
>>>>>
>>>>> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
>>>>>
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> This change was tried to push via JPRT, but it failed because
>>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSiz
>>>>>> eTest.java
>>>>>> was failed.
>>>>>> Also I heard the comment that the changes in arguments.cpp which set
>>>>>> ergonomic value to CompressedClassSpaceSize should be moved to
>>>>>> Metaspace::ergo_initialize().
>>>>>>
>>>>>> I uploaded new webrev. Could you review again?
>>>>>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
>>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile.
>>>>>>
>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
>>>>>>
>>>>>
>>>>>
>>>>> I'll let Coleen cover the technical details, I'll just pick up a few
>>>>> style
>>>>> nits:
>>>>>
>>>>> 3328   /* Initial virtual space size will be calculated at
>>>>> global_initialize() */
>>>>>
>>>>> Use // comment
>>>>>
>>>>> 3329   size_t min_metaspace_sz =
>>>>> 3330                    VIRTUALSPACEMULTIPLIER *
>>>>> InitialBootClassLoaderMetaspaceSize;
>>>>>
>>>>>              ^ should indent only to here
>>>>>
>>>>> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
>>>>> 3337                                       MaxMetaspaceSize -
>>>>> min_metaspace_sz);
>>>>>                              ^ should indent only to here
>>>>>
>>>>> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
>>>>> 3342 min_metaspace_sz);
>>>>>
>>>>>                           ^ should indent only to here
>>>>>
>>>>> You may have time to fix these in place before anyone else sees the
>>>>> webrev
>>>>> :)
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>
>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>>>>>>
>>>>>>>
>>>>>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it
>>>>>>> only
>>>>>>> needs
>>>>>>> one reviewer, unless there were more?  Please send me the result of
>>>>>>> hg
>>>>>>> export.
>>>>>>> thanks,
>>>>>>> Coleen
>>>>>>>
>>>>>>>
>>>>>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Coleen,
>>>>>>>>
>>>>>>>> I will sponsor this for you.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>>> check_metaspace_sizes()
>>>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>>>> is.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> I added new function calculate_min_metaspace_size() in metaspace.cpp
>>>>>>>> to get minimum metaspace size, and uses return value from this
>>>>>>>> function in metaspace initialization and metaspace size check.
>>>>>>>>
>>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>>>>>>
>>>>>>>> Could you review again?
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks for you both updating and sponsoring the webrev!
>>>>>>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>>>>>>
>>>>>>>>> -Man
>>>>>>>>>
>>>>>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>>> check_metaspace_sizes()
>>>>>>>>>> so that you don't have to tell arguments what the MediumChunk size
>>>>>>>>>> is.
>>>>>>>>>>
>>>>>>>>>> I will sponsor this for you.
>>>>>>>>>>
>>>>>>>>>> thanks,
>>>>>>>>>> Coleen
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> I added a testcase for this issue in new webrev:
>>>>>>>>>>>
>>>>>>>>>>>        http://cr.openjdk.java.net/~y
>>>>>>>>>>> suenaga/JDK-8087291/webrev.04/
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga <[hidden email]
>>>>>>>>>>> >:
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>
>>>>>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>
>>>>>>>>>>>>        http://cr.openjdk.java.net/~y
>>>>>>>>>>>> suenaga/JDK-8087291/webrev.03/
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Good to know that the patch is not blocked due to breaking some
>>>>>>>>>>>>> internal
>>>>>>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>>>>>>> Is it possible to push it to P4?
>>>>>>>>>>>>>
>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>>>>>>> <[hidden email]>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>>> class
>>>>>>>>>>>>>>> space
>>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs
>>>>>>>>>>>>>> after
>>>>>>>>>>>>>> repos
>>>>>>>>>>>>>> are opened.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Man,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Do you have any thoughts on why this patch has been pending
>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>> 2+
>>>>>>>>>>>>>>>> years? This patch could really save us from some annoying
>>>>>>>>>>>>>>>> issues
>>>>>>>>>>>>>>>> since we
>>>>>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>>> class
>>>>>>>>>>>>>>> space
>>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> StefanK
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>          Hi all,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>          I wonder if there is any recent update on the patch
>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>>>>>>          Is it possible to push this patch into JDK9? Except
>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>> its
>>>>>>>>>>>>>>>> low
>>>>>>>>>>>>>>>>          priority (P5),
>>>>>>>>>>>>>>>>          is there any complication that prevents this patch
>>>>>>>>>>>>>>>> getting
>>>>>>>>>>>>>>>> approved
>>>>>>>>>>>>>>>>          (for example, some JVM logic requires
>>>>>>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>          1GB by default)?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>          I work in the Java Platform Team at Google. We have
>>>>>>>>>>>>>>>> encountered
>>>>>>>>>>>>>>>>          annoying issues that the hsperfdata counter
>>>>>>>>>>>>>>>>          "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>>>>>>          a too large value (about 1GB) even if user sets
>>>>>>>>>>>>>>>>          -XX:MaxMetaspaceSize=100m, as well as GC log shows
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>>>>>>          memory reserved by metaspace,
>>>>>>>>>>>>>>>>          regardless of MaxMetaspaceSize value. The root
>>>>>>>>>>>>>>>> cause
>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>> these
>>>>>>>>>>>>>>>>          issues is that CompressedClassSpaceSize is not
>>>>>>>>>>>>>>>> automatically
>>>>>>>>>>>>>>>> capped
>>>>>>>>>>>>>>>>          by MaxMetaspaceSize
>>>>>>>>>>>>>>>>          during VM initialization, and this patch seems fix
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> root
>>>>>>>>>>>>>>>> cause.
>>>>>>>>>>>>>>>>          (I'm aware that even after this patch, the reserved
>>>>>>>>>>>>>>>> size
>>>>>>>>>>>>>>>> could
>>>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>>>          be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>>>>>>          but it is better than the current situation.)
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>          Thanks,
>>>>>>>>>>>>>>>>          Man
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>          On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>              Thank you for your comment!
>>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch with
>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>>              Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~y
>>>>>>>>>>>>>>>> suenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>>>>>>              It works on fastdebug VM.
>>>>>>>>>>>>>>>>              Please review again.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>              Thanks,
>>>>>>>>>>>>>>>>              Yasumasa
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>              On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>>>>>>               > Yasumasa,
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch with
>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               > On a linux system I get this when I build
>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>> your
>>>>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>>               >> # To suppress the following error report,
>>>>>>>>>>>>>>>> specify
>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>> argument
>>>>>>>>>>>>>>>>               >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>>>>>>              SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>>               >> # A fatal error has been detected by the
>>>>>>>>>>>>>>>> Java
>>>>>>>>>>>>>>>> Runtime
>>>>>>>>>>>>>>>>              Environment:
>>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>>               >> #  Internal Error
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-co
>>>>>>>>>>>>>>>> de_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>>>>>>               >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>>>>>>               >> #  assert(size > MediumChunk || size >
>>>>>>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>>>>>>              failed: Not a
>>>>>>>>>>>>>>>>               >> humongous chunk
>>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               > Jon
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>               > On 6/17/2015 7:54 AM, Yasumasa Suenaga
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>               >> I want to continue to discuss about
>>>>>>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>>>>>>              MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> http://mail.openjdk.java.net/p
>>>>>>>>>>>>>>>> ipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> <http://mail.openjdk.java.net/
>>>>>>>>>>>>>>>> pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>>>>>>               >>>> Should I resize CompressedClassSpaceSize
>>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>> show
>>>>>>>>>>>>>>>>              error message?
>>>>>>>>>>>>>>>>               >>> If you add slightly better heuristics for
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> setup
>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>              CompressedClassSpaceSize flag, for example
>>>>>>>>>>>>>>>> lowering
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>              CompressedClassSpaceSize when MaxMetaspaceSize
>>>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>> set,
>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>>              might be less likely that you'll hit the
>>>>>>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>>>>>>> when
>>>>>>>>>>>>>>>>              the system is set up with strict overcommit
>>>>>>>>>>>>>>>> settings.
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>               >> I've uploaded new webrev:
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~y
>>>>>>>>>>>>>>>> suenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>               >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>>>>>>              CompressedClassSpaceSize, and
>>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>               >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>>>>>>              Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>>>>>>               >> If InitialBootClassLoaderMetaspaceSize is
>>>>>>>>>>>>>>>> greater
>>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>>              MaxMetaspaceSize,
>>>>>>>>>>>>>>>>               >> VM will fail with error message.
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize will
>>>>>>>>>>>>>>>> be
>>>>>>>>>>>>>>>> set
>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>              MaxMetaspaceSize
>>>>>>>>>>>>>>>>               >> when UseCompressedClassPointers is not set
>>>>>>>>>>>>>>>> in
>>>>>>>>>>>>>>>>              Metaspace::ergo_initialize().
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>               >> Thanks,
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>               >> Yasumasa
>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Yasumasa Suenaga-4
Hi Man,

2017-11-10 11:31 GMT+09:00 Man Cao <[hidden email]>:

> Hi Yasumasa,
>
> I'm looking at if this patch can fix the original issue with hsperfdata
> counters sun_gc_metaspace_maxCapacity and
> sun_gc_compressedclassspace_maxCapacity being too large when user has set
> -XX:MaxMetaspaceSize=100m or some value less than 1GB. However, it does not,
> these counters still reports around 1GB if MaxMetaspaceSize is less than
> 1GB.
>
> The reason is that the code added in this patch to
> Metaspace::ergo_initialize()  is AFTER the two lines:
>
>  CompressedClassSpaceSize =
> align_size_down_bounded(CompressedClassSpaceSize, _reserve_alignment);
>  set_compressed_class_space_size(CompressedClassSpaceSize);
>
> So set_compressed_class_space_size() still gets the old value of
> CompressedClassSpaceSize that is too large, making the reserved size of
> metaspace too large.
>
> Should these two lines be moved after the code added in this patch?

I agree with you.
Can you send review request for it? According to OCA signatories [1],
Google signs OCA, so I think you can contribute it to OpenJDK.

I can file this to JBS, and create webrev and changest. But I cannot
push it to jdk/hs repo. So you need to find a sponsor.
I'm a reviewer of OpenJDK. So I can +1 to your change when you send
review request.


Thanks,

Yasumasa


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


> Thanks,
> -Man
>
> On Tue, Oct 17, 2017 at 12:46 PM, <[hidden email]> wrote:
>>
>>
>> Hi Yasumasa,
>> This looks good.  I can sponsor this for you.
>> thanks,
>> Coleen
>>
>>
>> On 10/12/17 1:16 AM, Yasumasa Suenaga wrote:
>>>
>>> Thanks David!
>>>
>>> I've fixed:
>>>
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.08/
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> 2017-10-12 13:55 GMT+09:00 David Holmes <[hidden email]>:
>>>>
>>>> On 12/10/2017 2:47 PM, Yasumasa Suenaga wrote:
>>>>>
>>>>> Hi David,
>>>>>
>>>>>> You may have time to fix these in place before anyone else sees the
>>>>>> webrev
>>>>>
>>>>>
>>>>> I've fixed that:
>>>>>
>>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/
>>>>>
>>>>>
>>>>> I checked raw text of your email, so I believe the indent is correct.
>>>>> If I have mistake(s), please tell me.
>>>>
>>>>
>>>> Sorry they are off. Basic rules:
>>>>
>>>> var =
>>>>      some long value;
>>>>
>>>> indent by 4.
>>>>
>>>> foo(arg1, arg2,
>>>>      arg3, arg4);
>>>>
>>>> align the args on the two lines as shown.
>>>>
>>>> Hope that is clearer.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> 2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:
>>>>>>
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> This change was tried to push via JPRT, but it failed because
>>>>>>>
>>>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile/MaxMetaspaceSizeTest.java
>>>>>>> was failed.
>>>>>>> Also I heard the comment that the changes in arguments.cpp which set
>>>>>>> ergonomic value to CompressedClassSpaceSize should be moved to
>>>>>>> Metaspace::ergo_initialize().
>>>>>>>
>>>>>>> I uploaded new webrev. Could you review again?
>>>>>>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
>>>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile.
>>>>>>>
>>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
>>>>>>
>>>>>>
>>>>>>
>>>>>> I'll let Coleen cover the technical details, I'll just pick up a few
>>>>>> style
>>>>>> nits:
>>>>>>
>>>>>> 3328   /* Initial virtual space size will be calculated at
>>>>>> global_initialize() */
>>>>>>
>>>>>> Use // comment
>>>>>>
>>>>>> 3329   size_t min_metaspace_sz =
>>>>>> 3330                    VIRTUALSPACEMULTIPLIER *
>>>>>> InitialBootClassLoaderMetaspaceSize;
>>>>>>
>>>>>>              ^ should indent only to here
>>>>>>
>>>>>> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
>>>>>> 3337                                       MaxMetaspaceSize -
>>>>>> min_metaspace_sz);
>>>>>>                              ^ should indent only to here
>>>>>>
>>>>>> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
>>>>>> 3342 min_metaspace_sz);
>>>>>>
>>>>>>                           ^ should indent only to here
>>>>>>
>>>>>> You may have time to fix these in place before anyone else sees the
>>>>>> webrev
>>>>>> :)
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it
>>>>>>>> only
>>>>>>>> needs
>>>>>>>> one reviewer, unless there were more?  Please send me the result of
>>>>>>>> hg
>>>>>>>> export.
>>>>>>>> thanks,
>>>>>>>> Coleen
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Coleen,
>>>>>>>>>
>>>>>>>>>>> I will sponsor this for you.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks!
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>>>> check_metaspace_sizes()
>>>>>>>>>>> so that you don't have to tell arguments what the MediumChunk
>>>>>>>>>>> size
>>>>>>>>>>> is.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I added new function calculate_min_metaspace_size() in
>>>>>>>>> metaspace.cpp
>>>>>>>>> to get minimum metaspace size, and uses return value from this
>>>>>>>>> function in metaspace initialization and metaspace size check.
>>>>>>>>>
>>>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.05/
>>>>>>>>>
>>>>>>>>> Could you review again?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks for you both updating and sponsoring the webrev!
>>>>>>>>>> We plan to back-port it to our JDK9 once it is submitted.
>>>>>>>>>>
>>>>>>>>>> -Man
>>>>>>>>>>
>>>>>>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
>>>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
>>>>>>>>>>> check_metaspace_sizes()
>>>>>>>>>>> so that you don't have to tell arguments what the MediumChunk
>>>>>>>>>>> size
>>>>>>>>>>> is.
>>>>>>>>>>>
>>>>>>>>>>> I will sponsor this for you.
>>>>>>>>>>>
>>>>>>>>>>> thanks,
>>>>>>>>>>> Coleen
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>
>>>>>>>>>>>> I added a testcase for this issue in new webrev:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga
>>>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
>>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thank Yasumasa and Stefan for the responses.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Good to know that the patch is not blocked due to breaking
>>>>>>>>>>>>>> some
>>>>>>>>>>>>>> internal
>>>>>>>>>>>>>> invariants/assumptions, but just due to its P5 status.
>>>>>>>>>>>>>> Is it possible to push it to P4?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
>>>>>>>>>>>>>> <[hidden email]>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> (CC'ed hotspot-runtime-dev)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>>>> class
>>>>>>>>>>>>>>>> space
>>>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction
>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs
>>>>>>>>>>>>>>> after
>>>>>>>>>>>>>>> repos
>>>>>>>>>>>>>>> are opened.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi Man,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi Yasumasa, Stefan,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Do you have any thoughts on why this patch has been pending
>>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>> 2+
>>>>>>>>>>>>>>>>> years? This patch could really save us from some annoying
>>>>>>>>>>>>>>>>> issues
>>>>>>>>>>>>>>>>> since we
>>>>>>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The compressed
>>>>>>>>>>>>>>>> class
>>>>>>>>>>>>>>>> space
>>>>>>>>>>>>>>>> belongs to the runtime code, so you might get more traction
>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>> on the
>>>>>>>>>>>>>>>> hotspot-runtime-dev list.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> StefanK
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> -Man
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <[hidden email]
>>>>>>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>          Hi all,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>          I wonder if there is any recent update on the
>>>>>>>>>>>>>>>>> patch
>>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>> JDK-8087291.
>>>>>>>>>>>>>>>>>          Is it possible to push this patch into JDK9?
>>>>>>>>>>>>>>>>> Except
>>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>> its
>>>>>>>>>>>>>>>>> low
>>>>>>>>>>>>>>>>>          priority (P5),
>>>>>>>>>>>>>>>>>          is there any complication that prevents this patch
>>>>>>>>>>>>>>>>> getting
>>>>>>>>>>>>>>>>> approved
>>>>>>>>>>>>>>>>>          (for example, some JVM logic requires
>>>>>>>>>>>>>>>>> CompressedClassSpaceSize
>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>>          1GB by default)?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>          I work in the Java Platform Team at Google. We
>>>>>>>>>>>>>>>>> have
>>>>>>>>>>>>>>>>> encountered
>>>>>>>>>>>>>>>>>          annoying issues that the hsperfdata counter
>>>>>>>>>>>>>>>>>          "sun_gc_metaspace_maxCapacity" reporting
>>>>>>>>>>>>>>>>>          a too large value (about 1GB) even if user sets
>>>>>>>>>>>>>>>>>          -XX:MaxMetaspaceSize=100m, as well as GC log shows
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> confusing 1GB
>>>>>>>>>>>>>>>>>          memory reserved by metaspace,
>>>>>>>>>>>>>>>>>          regardless of MaxMetaspaceSize value. The root
>>>>>>>>>>>>>>>>> cause
>>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>> these
>>>>>>>>>>>>>>>>>          issues is that CompressedClassSpaceSize is not
>>>>>>>>>>>>>>>>> automatically
>>>>>>>>>>>>>>>>> capped
>>>>>>>>>>>>>>>>>          by MaxMetaspaceSize
>>>>>>>>>>>>>>>>>          during VM initialization, and this patch seems fix
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> root
>>>>>>>>>>>>>>>>> cause.
>>>>>>>>>>>>>>>>>          (I'm aware that even after this patch, the
>>>>>>>>>>>>>>>>> reserved
>>>>>>>>>>>>>>>>> size
>>>>>>>>>>>>>>>>> could
>>>>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>>>>          be up to 2*MaxMetaspaceSize,
>>>>>>>>>>>>>>>>>          but it is better than the current situation.)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>          Thanks,
>>>>>>>>>>>>>>>>>          Man
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>          On 6/19/2015 00:34, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>              Thank you for your comment!
>>>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch
>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>>>              Sorry, I've fixed it and uploaded webrev:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.02/>
>>>>>>>>>>>>>>>>>              It works on fastdebug VM.
>>>>>>>>>>>>>>>>>              Please review again.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>              Thanks,
>>>>>>>>>>>>>>>>>              Yasumasa
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>              On 2015/06/18 10:45, Jon Masamitsu wrote:
>>>>>>>>>>>>>>>>>               > Yasumasa,
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch
>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>> command
>>>>>>>>>>>>>>>>> line.
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               > On a linux system I get this when I build
>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>> your
>>>>>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               >> java -XX:MaxMetaspaceSize=4195328 -version
>>>>>>>>>>>>>>>>>               >> # To suppress the following error report,
>>>>>>>>>>>>>>>>> specify
>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>> argument
>>>>>>>>>>>>>>>>>               >> # after -XX: or in .hotspotrc:
>>>>>>>>>>>>>>>>>              SuppressErrorAt=/metaspace.cpp:2324
>>>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>>>               >> # A fatal error has been detected by the
>>>>>>>>>>>>>>>>> Java
>>>>>>>>>>>>>>>>> Runtime
>>>>>>>>>>>>>>>>>              Environment:
>>>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>>>               >> #  Internal Error
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/memory/metaspace.cpp:2324),
>>>>>>>>>>>>>>>>>               >> pid=19099, tid=0x00007ff4b9b92700
>>>>>>>>>>>>>>>>>               >> #  assert(size > MediumChunk || size >
>>>>>>>>>>>>>>>>> ClassMediumChunk)
>>>>>>>>>>>>>>>>>              failed: Not a
>>>>>>>>>>>>>>>>>               >> humongous chunk
>>>>>>>>>>>>>>>>>               >> #
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               > Jon
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               >
>>>>>>>>>>>>>>>>>               > On 6/17/2015 7:54 AM, Yasumasa Suenaga
>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>               >> I want to continue to discuss about
>>>>>>>>>>>>>>>>> CompressedClassSpace and
>>>>>>>>>>>>>>>>>              MaxMetaspace in this (RFR) thread.
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013873.html>
>>>>>>>>>>>>>>>>>               >>>> Should I resize CompressedClassSpaceSize
>>>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>> show
>>>>>>>>>>>>>>>>>              error message?
>>>>>>>>>>>>>>>>>               >>> If you add slightly better heuristics for
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> setup
>>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>              CompressedClassSpaceSize flag, for example
>>>>>>>>>>>>>>>>> lowering
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>              CompressedClassSpaceSize when MaxMetaspaceSize
>>>>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>> set,
>>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>>>              might be less likely that you'll hit the
>>>>>>>>>>>>>>>>> OutOfMemoryError
>>>>>>>>>>>>>>>>> when
>>>>>>>>>>>>>>>>>              the system is set up with strict overcommit
>>>>>>>>>>>>>>>>> settings.
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>               >> I've uploaded new webrev:
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.01/>
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>               >> This patch checkes MaxMetaspaceSize,
>>>>>>>>>>>>>>>>>              CompressedClassSpaceSize, and
>>>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize.
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>               >> I add to check CompressedClassSpaceSize in
>>>>>>>>>>>>>>>>>              Arguments::set_use_compressed_klass_ptrs().
>>>>>>>>>>>>>>>>>               >> If InitialBootClassLoaderMetaspaceSize is
>>>>>>>>>>>>>>>>> greater
>>>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>>>              MaxMetaspaceSize,
>>>>>>>>>>>>>>>>>               >> VM will fail with error message.
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize will
>>>>>>>>>>>>>>>>> be
>>>>>>>>>>>>>>>>> set
>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>              MaxMetaspaceSize
>>>>>>>>>>>>>>>>>               >> when UseCompressedClassPointers is not set
>>>>>>>>>>>>>>>>> in
>>>>>>>>>>>>>>>>>              Metaspace::ergo_initialize().
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>               >> Thanks,
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>               >> Yasumasa
>>>>>>>>>>>>>>>>>               >>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize

Man Cao
Hi Yasumasa,

Yes, that sounds good. I haven't submitted any code to OpenJDK yet. This is
a good starter patch to get me familiar with the process.

-Man

On Fri, Nov 10, 2017 at 12:15 AM, Yasumasa Suenaga <[hidden email]>
wrote:

> Hi Man,
>
> 2017-11-10 11:31 GMT+09:00 Man Cao <[hidden email]>:
> > Hi Yasumasa,
> >
> > I'm looking at if this patch can fix the original issue with hsperfdata
> > counters sun_gc_metaspace_maxCapacity and
> > sun_gc_compressedclassspace_maxCapacity being too large when user has
> set
> > -XX:MaxMetaspaceSize=100m or some value less than 1GB. However, it does
> not,
> > these counters still reports around 1GB if MaxMetaspaceSize is less than
> > 1GB.
> >
> > The reason is that the code added in this patch to
> > Metaspace::ergo_initialize()  is AFTER the two lines:
> >
> >  CompressedClassSpaceSize =
> > align_size_down_bounded(CompressedClassSpaceSize, _reserve_alignment);
> >  set_compressed_class_space_size(CompressedClassSpaceSize);
> >
> > So set_compressed_class_space_size() still gets the old value of
> > CompressedClassSpaceSize that is too large, making the reserved size of
> > metaspace too large.
> >
> > Should these two lines be moved after the code added in this patch?
>
> I agree with you.
> Can you send review request for it? According to OCA signatories [1],
> Google signs OCA, so I think you can contribute it to OpenJDK.
>
> I can file this to JBS, and create webrev and changest. But I cannot
> push it to jdk/hs repo. So you need to find a sponsor.
> I'm a reviewer of OpenJDK. So I can +1 to your change when you send
> review request.
>
>
> Thanks,
>
> Yasumasa
>
>
> [1] http://www.oracle.com/technetwork/community/oca-486395.html
>
>
> > Thanks,
> > -Man
> >
> > On Tue, Oct 17, 2017 at 12:46 PM, <[hidden email]> wrote:
> >>
> >>
> >> Hi Yasumasa,
> >> This looks good.  I can sponsor this for you.
> >> thanks,
> >> Coleen
> >>
> >>
> >> On 10/12/17 1:16 AM, Yasumasa Suenaga wrote:
> >>>
> >>> Thanks David!
> >>>
> >>> I've fixed:
> >>>
> >>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.08/
> >>>
> >>>
> >>> Yasumasa
> >>>
> >>>
> >>> 2017-10-12 13:55 GMT+09:00 David Holmes <[hidden email]>:
> >>>>
> >>>> On 12/10/2017 2:47 PM, Yasumasa Suenaga wrote:
> >>>>>
> >>>>> Hi David,
> >>>>>
> >>>>>> You may have time to fix these in place before anyone else sees the
> >>>>>> webrev
> >>>>>
> >>>>>
> >>>>> I've fixed that:
> >>>>>
> >>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.07/
> >>>>>
> >>>>>
> >>>>> I checked raw text of your email, so I believe the indent is correct.
> >>>>> If I have mistake(s), please tell me.
> >>>>
> >>>>
> >>>> Sorry they are off. Basic rules:
> >>>>
> >>>> var =
> >>>>      some long value;
> >>>>
> >>>> indent by 4.
> >>>>
> >>>> foo(arg1, arg2,
> >>>>      arg3, arg4);
> >>>>
> >>>> align the args on the two lines as shown.
> >>>>
> >>>> Hope that is clearer.
> >>>>
> >>>> Thanks,
> >>>> David
> >>>>
> >>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Yasumasa
> >>>>>
> >>>>>
> >>>>> 2017-10-12 13:22 GMT+09:00 David Holmes <[hidden email]>:
> >>>>>>
> >>>>>> Hi Yasumasa,
> >>>>>>
> >>>>>> On 12/10/2017 2:10 PM, Yasumasa Suenaga wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> This change was tried to push via JPRT, but it failed because
> >>>>>>>
> >>>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile/
> MaxMetaspaceSizeTest.java
> >>>>>>> was failed.
> >>>>>>> Also I heard the comment that the changes in arguments.cpp which
> set
> >>>>>>> ergonomic value to CompressedClassSpaceSize should be moved to
> >>>>>>> Metaspace::ergo_initialize().
> >>>>>>>
> >>>>>>> I uploaded new webrev. Could you review again?
> >>>>>>> This webrev works fine on test/hotspot/jtreg/runtime/Metaspace and
> >>>>>>> test/hotspot/jtreg/runtime/SharedArchiveFile.
> >>>>>>>
> >>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.06/
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> I'll let Coleen cover the technical details, I'll just pick up a few
> >>>>>> style
> >>>>>> nits:
> >>>>>>
> >>>>>> 3328   /* Initial virtual space size will be calculated at
> >>>>>> global_initialize() */
> >>>>>>
> >>>>>> Use // comment
> >>>>>>
> >>>>>> 3329   size_t min_metaspace_sz =
> >>>>>> 3330                    VIRTUALSPACEMULTIPLIER *
> >>>>>> InitialBootClassLoaderMetaspaceSize;
> >>>>>>
> >>>>>>              ^ should indent only to here
> >>>>>>
> >>>>>> 3336         FLAG_SET_ERGO(size_t, CompressedClassSpaceSize,
> >>>>>> 3337                                       MaxMetaspaceSize -
> >>>>>> min_metaspace_sz);
> >>>>>>                              ^ should indent only to here
> >>>>>>
> >>>>>> 3341     FLAG_SET_ERGO(size_t, InitialBootClassLoaderMetaspaceSize,
> >>>>>> 3342 min_metaspace_sz);
> >>>>>>
> >>>>>>                           ^ should indent only to here
> >>>>>>
> >>>>>> You may have time to fix these in place before anyone else sees the
> >>>>>> webrev
> >>>>>> :)
> >>>>>>
> >>>>>> Thanks,
> >>>>>> David
> >>>>>> -----
> >>>>>>
> >>>>>>
> >>>>>>> Thanks,
> >>>>>>>
> >>>>>>> Yasumasa
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> 2017-10-10 21:02 GMT+09:00  <[hidden email]>:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Hi Yasumasa,  I like this better.  I will sponsor it.  I think it
> >>>>>>>> only
> >>>>>>>> needs
> >>>>>>>> one reviewer, unless there were more?  Please send me the result
> of
> >>>>>>>> hg
> >>>>>>>> export.
> >>>>>>>> thanks,
> >>>>>>>> Coleen
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 10/9/17 11:09 PM, Yasumasa Suenaga wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Hi Coleen,
> >>>>>>>>>
> >>>>>>>>>>> I will sponsor this for you.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks!
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
> >>>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
> >>>>>>>>>>> check_metaspace_sizes()
> >>>>>>>>>>> so that you don't have to tell arguments what the MediumChunk
> >>>>>>>>>>> size
> >>>>>>>>>>> is.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> I added new function calculate_min_metaspace_size() in
> >>>>>>>>> metaspace.cpp
> >>>>>>>>> to get minimum metaspace size, and uses return value from this
> >>>>>>>>> function in metaspace initialization and metaspace size check.
> >>>>>>>>>
> >>>>>>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.
> 05/
> >>>>>>>>>
> >>>>>>>>> Could you review again?
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>>
> >>>>>>>>> Yasumasa
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> 2017-10-10 6:07 GMT+09:00 Man Cao <[hidden email]>:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Thanks for you both updating and sponsoring the webrev!
> >>>>>>>>>> We plan to back-port it to our JDK9 once it is submitted.
> >>>>>>>>>>
> >>>>>>>>>> -Man
> >>>>>>>>>>
> >>>>>>>>>> On Mon, Oct 9, 2017 at 6:15 AM, <[hidden email]>
> >>>>>>>>>> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> Hi, this looks reasonable but I would prefer that the code in
> >>>>>>>>>>> arguments.cpp call something in metaspace.cpp, like
> >>>>>>>>>>> check_metaspace_sizes()
> >>>>>>>>>>> so that you don't have to tell arguments what the MediumChunk
> >>>>>>>>>>> size
> >>>>>>>>>>> is.
> >>>>>>>>>>>
> >>>>>>>>>>> I will sponsor this for you.
> >>>>>>>>>>>
> >>>>>>>>>>> thanks,
> >>>>>>>>>>> Coleen
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On 9/26/17 11:05 PM, Yasumasa Suenaga wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Hi all,
> >>>>>>>>>>>>
> >>>>>>>>>>>> I added a testcase for this issue in new webrev:
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.04/
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>
> >>>>>>>>>>>> Yasumasa
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> 2017-09-26 18:36 GMT+09:00 Yasumasa Suenaga
> >>>>>>>>>>>> <[hidden email]>:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Hi all,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I uploaded webrev for this issue against jdk10/hs.
> >>>>>>>>>>>>> Could you review it?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.03/
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Yasumasa
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 2017-09-21 3:11 GMT+09:00 Man Cao <[hidden email]>:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Thank Yasumasa and Stefan for the responses.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Good to know that the patch is not blocked due to breaking
> >>>>>>>>>>>>>> some
> >>>>>>>>>>>>>> internal
> >>>>>>>>>>>>>> invariants/assumptions, but just due to its P5 status.
> >>>>>>>>>>>>>> Is it possible to push it to P4?
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> -Man
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On Wed, Sep 20, 2017 at 5:16 AM, Yasumasa Suenaga
> >>>>>>>>>>>>>> <[hidden email]>
> >>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Hi,
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> (CC'ed hotspot-runtime-dev)
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The
> compressed
> >>>>>>>>>>>>>>>> class
> >>>>>>>>>>>>>>>> space
> >>>>>>>>>>>>>>>> belongs to the runtime code, so you might get more
> traction
> >>>>>>>>>>>>>>>> for
> >>>>>>>>>>>>>>>> this
> >>>>>>>>>>>>>>>> on the
> >>>>>>>>>>>>>>>> hotspot-runtime-dev list.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> I will send review request against jdk10/master or jdk10/hs
> >>>>>>>>>>>>>>> after
> >>>>>>>>>>>>>>> repos
> >>>>>>>>>>>>>>> are opened.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Yasumasa
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> On 2017/09/20 20:53, Stefan Karlsson wrote:
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Hi Man,
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> On 2017-09-13 20:55, Man Cao wrote:
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> Hi Yasumasa, Stefan,
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> Do you have any thoughts on why this patch has been
> pending
> >>>>>>>>>>>>>>>>> for
> >>>>>>>>>>>>>>>>> 2+
> >>>>>>>>>>>>>>>>> years? This patch could really save us from some annoying
> >>>>>>>>>>>>>>>>> issues
> >>>>>>>>>>>>>>>>> since we
> >>>>>>>>>>>>>>>>> are automatically monitoring hsperfdata counters.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> I think the reason is that this bug is a P5. The
> compressed
> >>>>>>>>>>>>>>>> class
> >>>>>>>>>>>>>>>> space
> >>>>>>>>>>>>>>>> belongs to the runtime code, so you might get more
> traction
> >>>>>>>>>>>>>>>> for
> >>>>>>>>>>>>>>>> this
> >>>>>>>>>>>>>>>> on the
> >>>>>>>>>>>>>>>> hotspot-runtime-dev list.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> StefanK
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> -Man
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> On Mon, Aug 21, 2017 at 3:46 PM, Man Cao <
> [hidden email]
> >>>>>>>>>>>>>>>>> <mailto:[hidden email]>> wrote:
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>          Hi all,
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>          I wonder if there is any recent update on the
> >>>>>>>>>>>>>>>>> patch
> >>>>>>>>>>>>>>>>> for
> >>>>>>>>>>>>>>>>> JDK-8087291.
> >>>>>>>>>>>>>>>>>          Is it possible to push this patch into JDK9?
> >>>>>>>>>>>>>>>>> Except
> >>>>>>>>>>>>>>>>> for
> >>>>>>>>>>>>>>>>> its
> >>>>>>>>>>>>>>>>> low
> >>>>>>>>>>>>>>>>>          priority (P5),
> >>>>>>>>>>>>>>>>>          is there any complication that prevents this
> patch
> >>>>>>>>>>>>>>>>> getting
> >>>>>>>>>>>>>>>>> approved
> >>>>>>>>>>>>>>>>>          (for example, some JVM logic requires
> >>>>>>>>>>>>>>>>> CompressedClassSpaceSize
> >>>>>>>>>>>>>>>>> to be
> >>>>>>>>>>>>>>>>>          1GB by default)?
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>          I work in the Java Platform Team at Google. We
> >>>>>>>>>>>>>>>>> have
> >>>>>>>>>>>>>>>>> encountered
> >>>>>>>>>>>>>>>>>          annoying issues that the hsperfdata counter
> >>>>>>>>>>>>>>>>>          "sun_gc_metaspace_maxCapacity" reporting
> >>>>>>>>>>>>>>>>>          a too large value (about 1GB) even if user sets
> >>>>>>>>>>>>>>>>>          -XX:MaxMetaspaceSize=100m, as well as GC log
> shows
> >>>>>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>> confusing 1GB
> >>>>>>>>>>>>>>>>>          memory reserved by metaspace,
> >>>>>>>>>>>>>>>>>          regardless of MaxMetaspaceSize value. The root
> >>>>>>>>>>>>>>>>> cause
> >>>>>>>>>>>>>>>>> for
> >>>>>>>>>>>>>>>>> these
> >>>>>>>>>>>>>>>>>          issues is that CompressedClassSpaceSize is not
> >>>>>>>>>>>>>>>>> automatically
> >>>>>>>>>>>>>>>>> capped
> >>>>>>>>>>>>>>>>>          by MaxMetaspaceSize
> >>>>>>>>>>>>>>>>>          during VM initialization, and this patch seems
> fix
> >>>>>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>> root
> >>>>>>>>>>>>>>>>> cause.
> >>>>>>>>>>>>>>>>>          (I'm aware that even after this patch, the
> >>>>>>>>>>>>>>>>> reserved
> >>>>>>>>>>>>>>>>> size
> >>>>>>>>>>>>>>>>> could
> >>>>>>>>>>>>>>>>> still
> >>>>>>>>>>>>>>>>>          be up to 2*MaxMetaspaceSize,
> >>>>>>>>>>>>>>>>>          but it is better than the current situation.)
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>          Thanks,
> >>>>>>>>>>>>>>>>>          Man
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>          On 6/19/2015 00:34, Yasumasa Suenaga wrote:
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>              Thank you for your comment!
> >>>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch
> >>>>>>>>>>>>>>>>> with
> >>>>>>>>>>>>>>>>> this
> >>>>>>>>>>>>>>>>> command
> >>>>>>>>>>>>>>>>> line.
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328
> -version
> >>>>>>>>>>>>>>>>>              Sorry, I've fixed it and uploaded webrev:
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.
> 02/
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~
> ysuenaga/JDK-8087291/webrev.02/>
> >>>>>>>>>>>>>>>>>              It works on fastdebug VM.
> >>>>>>>>>>>>>>>>>              Please review again.
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>              Thanks,
> >>>>>>>>>>>>>>>>>              Yasumasa
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>              On 2015/06/18 10:45, Jon Masamitsu wrote:
> >>>>>>>>>>>>>>>>>               > Yasumasa,
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               > Try running a debug JVM with your patch
> >>>>>>>>>>>>>>>>> with
> >>>>>>>>>>>>>>>>> this
> >>>>>>>>>>>>>>>>> command
> >>>>>>>>>>>>>>>>> line.
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               > java -XX:MaxMetaspaceSize=4195328
> -version
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               > On a linux system I get this when I build
> >>>>>>>>>>>>>>>>> with
> >>>>>>>>>>>>>>>>> your
> >>>>>>>>>>>>>>>>> patch.
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               >> java -XX:MaxMetaspaceSize=4195328
> -version
> >>>>>>>>>>>>>>>>>               >> # To suppress the following error
> report,
> >>>>>>>>>>>>>>>>> specify
> >>>>>>>>>>>>>>>>> this
> >>>>>>>>>>>>>>>>> argument
> >>>>>>>>>>>>>>>>>               >> # after -XX: or in .hotspotrc:
> >>>>>>>>>>>>>>>>>              SuppressErrorAt=/metaspace.cpp:2324
> >>>>>>>>>>>>>>>>>               >> #
> >>>>>>>>>>>>>>>>>               >> # A fatal error has been detected by the
> >>>>>>>>>>>>>>>>> Java
> >>>>>>>>>>>>>>>>> Runtime
> >>>>>>>>>>>>>>>>>              Environment:
> >>>>>>>>>>>>>>>>>               >> #
> >>>>>>>>>>>>>>>>>               >> #  Internal Error
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> (/export/jmasa/java/jdk9-gc-code_review/src/share/vm/
> memory/metaspace.cpp:2324),
> >>>>>>>>>>>>>>>>>               >> pid=19099, tid=0x00007ff4b9b92700
> >>>>>>>>>>>>>>>>>               >> #  assert(size > MediumChunk || size >
> >>>>>>>>>>>>>>>>> ClassMediumChunk)
> >>>>>>>>>>>>>>>>>              failed: Not a
> >>>>>>>>>>>>>>>>>               >> humongous chunk
> >>>>>>>>>>>>>>>>>               >> #
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               > Jon
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               >
> >>>>>>>>>>>>>>>>>               > On 6/17/2015 7:54 AM, Yasumasa Suenaga
> >>>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>               >> I want to continue to discuss about
> >>>>>>>>>>>>>>>>> CompressedClassSpace and
> >>>>>>>>>>>>>>>>>              MaxMetaspace in this (RFR) thread.
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> http://mail.openjdk.java.net/
> pipermail/hotspot-gc-dev/2015-June/013873.html
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> <http://mail.openjdk.java.net/
> pipermail/hotspot-gc-dev/2015-June/013873.html>
> >>>>>>>>>>>>>>>>>               >>>> Should I resize
> CompressedClassSpaceSize
> >>>>>>>>>>>>>>>>> than
> >>>>>>>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>> show
> >>>>>>>>>>>>>>>>>              error message?
> >>>>>>>>>>>>>>>>>               >>> If you add slightly better heuristics
> for
> >>>>>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>> setup
> >>>>>>>>>>>>>>>>> of
> >>>>>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>>              CompressedClassSpaceSize flag, for example
> >>>>>>>>>>>>>>>>> lowering
> >>>>>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>>              CompressedClassSpaceSize when
> MaxMetaspaceSize
> >>>>>>>>>>>>>>>>> is
> >>>>>>>>>>>>>>>>> set,
> >>>>>>>>>>>>>>>>> then
> >>>>>>>>>>>>>>>>> it
> >>>>>>>>>>>>>>>>>              might be less likely that you'll hit the
> >>>>>>>>>>>>>>>>> OutOfMemoryError
> >>>>>>>>>>>>>>>>> when
> >>>>>>>>>>>>>>>>>              the system is set up with strict overcommit
> >>>>>>>>>>>>>>>>> settings.
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>               >> I've uploaded new webrev:
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8087291/webrev.
> 01/
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~
> ysuenaga/JDK-8087291/webrev.01/>
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>               >> This patch checkes MaxMetaspaceSize,
> >>>>>>>>>>>>>>>>>              CompressedClassSpaceSize, and
> >>>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize.
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>               >> I add to check CompressedClassSpaceSize
> in
> >>>>>>>>>>>>>>>>>              Arguments::set_use_compressed_klass_ptrs().
> >>>>>>>>>>>>>>>>>               >> If InitialBootClassLoaderMetaspaceSize
> is
> >>>>>>>>>>>>>>>>> greater
> >>>>>>>>>>>>>>>>> than
> >>>>>>>>>>>>>>>>>              MaxMetaspaceSize,
> >>>>>>>>>>>>>>>>>               >> VM will fail with error message.
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>               >> InitialBootClassLoaderMetaspaceSize
> will
> >>>>>>>>>>>>>>>>> be
> >>>>>>>>>>>>>>>>> set
> >>>>>>>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>>              MaxMetaspaceSize
> >>>>>>>>>>>>>>>>>               >> when UseCompressedClassPointers is not
> set
> >>>>>>>>>>>>>>>>> in
> >>>>>>>>>>>>>>>>>              Metaspace::ergo_initialize().
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>               >> Thanks,
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>               >> Yasumasa
> >>>>>>>>>>>>>>>>>               >>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>
> >
>