RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Tue Oct 17 19:46:17 UTC 2017
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 <david.holmes at oracle.com>:
>> 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 <david.holmes at oracle.com>:
>>>> 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 <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