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