RFR: JDK-8087291: InitialBootClassLoaderMetaspaceSize and CompressedClassSpaceSize should be checked consistent from MaxMetaspaceSize
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Tue Oct 10 12:02:31 UTC 2017
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