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

Yasumasa Suenaga yasuenag at gmail.com
Thu Oct 12 04:10:58 UTC 2017


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  <coleen.phillimore at oracle.com>:
> 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 <manc at google.com>:
>>>
>>> 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, <coleen.phillimore at oracle.com> 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 <yasuenag at gmail.com>:
>>>>>>
>>>>>> 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 <manc at google.com>:
>>>>>>>
>>>>>>> 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
>>>>>>> <yasuenag at gmail.com>
>>>>>>> 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 <manc at google.com
>>>>>>>>>> <mailto:manc at google.com>> 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
>>>>>>>>>>            >>
>>>>>>>>>>
>>>>>>>>>>
>


More information about the hotspot-runtime-dev mailing list