-XX:MetaspaceSize is correct?

[Yasumasa Suenaga]

I uploaded webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8039867/webrev.00/

Please review and sponsoring!


Yasumasa


On 04/10/2014 12:37 PM, Yasumasa Suenaga wrote:
> Hi Jon,
> Thank you for replying.
>
> I filed this issue as JDK-8039867: Incorrect description: -XX:MetaspaceSize .
>
> I attached a patch to this entry.
> I will upload webrev later.
>
>
> Thanks,
>
> Yasumasa
>
>
>
> 2014-04-10 6:51 GMT+09:00, Jon Masamitsu <jon.masamitsu at oracle.com>:
>> On 04/09/2014 06:22 AM, Yasumasa Suenaga wrote:
>>> Hi all,
>>>
>>> I checked initial metaspace size through jcmd PerfCounter.print .
>>> However, it seems to be incorrect:
>>>
>>>   - Java command:
>>>    java -XX:-UseCompressedClassPointers -XX:+PrintFlagsFinal LongSleep
>>>
>>>   - Output from -XX:+PrintFlagsFinal:
>>>      uintx MetaspaceSize                             =
>>> 21807104                            {pd product}
>>>
>>>   - Result of "PerfCounter.print"
>>> sun.gc.metaspace.capacity=4194304
>>> sun.gc.metaspace.maxCapacity=8388608
>>> sun.gc.metaspace.minCapacity=0
>>>
>>>
>>> I checked metaspace.cpp, initial size of metaspace is detected by
>>> InitialBootClassLoaderMetaspaceSize.
>>> However, description of MetaspaceSize in globals.hpp is
>>> "Initial size of Metaspaces (in bytes)" .
>>>
>>> Is description of MetaspaceSize is correct?
>> Description is not correct.
>>
>>> And this behavior is correct?
>> Behavior is correct.
>>
>>> I have two plan for this mismatch:
>>>
>>>   A) Change description of MetaspaceSize
>>>    MetaspaceSize is used to HWM in metaspace.cpp .
>>>    So we should change description for this behavior.
>>> ------------
>>> diff -r 48ce2e6e1add src/share/vm/runtime/globals.hpp
>>> --- a/src/share/vm/runtime/globals.hpp  Fri Apr 04 10:04:44 2014 -0700
>>> +++ b/src/share/vm/runtime/globals.hpp  Wed Apr 09 22:05:18 2014 +0900
>>> @@ -3160,7 +3156,7 @@
>>>             "non-daemon thread (in
>>> bytes)")                                   \
>>> \
>>>     product_pd(uintx,
>>> MetaspaceSize,                                          \
>>> -          "Initial size of Metaspaces (in
>>> bytes)")                          \
>>> +          "Initial HWM of Metaspaces (in
>>> bytes)")                           \
>> Explain what HWM is if you're going to use it.
>>
>>> \
>>>     product(uintx, MaxMetaspaceSize,
>>> max_uintx,                               \
>>>             "Maximum size of Metaspaces (in
>>> bytes)")                          \
>>> ------------
>>>
>>>   B) Remove InitialBootClassLoaderMetaspaceSize and use MetaspaceSize
>>>    In currently, InitialBootClassLoaderMetaspaceSize is used to initialize
>>>    metaspace.
>>>    InitialBootClassLoaderMetaspaceSize is only to use for it.
>>>    Thus we should remove this option and use MetaspaceSize to initialize
>>>    metaspace.
>> InitialBootClassLoaderMetaspaceSize is an optimization.  It allows
>> approximately
>> enough space for the system classes without repeated  allocations of
>> Metaspaces.
>> Not everyone agrees with this optimization but I would like it kept.
>>
>>> ------------
>>> diff -r 48ce2e6e1add src/share/vm/memory/metaspace.cpp
>>> --- a/src/share/vm/memory/metaspace.cpp Fri Apr 04 10:04:44 2014 -0700
>>> +++ b/src/share/vm/memory/metaspace.cpp Wed Apr 09 22:05:18 2014 +0900
>>> @@ -3172,7 +3172,7 @@
>>>   #endif
>>>
>>>       // Initialize these before initializing the VirtualSpaceList
>>> -    _first_chunk_word_size = InitialBootClassLoaderMetaspaceSize /
>>> BytesPerWord;
>>> +    _first_chunk_word_size = MetaspaceSize / BytesPerWord;
>>>       _first_chunk_word_size = align_word_size_up(_first_chunk_word_size);
>>>       // Make the first class chunk bigger than a medium chunk so it's
>>> not put
>>>       // on the medium chunk list.   The next chunk will be small and
>>> progress
>>> diff -r 48ce2e6e1add src/share/vm/runtime/globals.hpp
>>> --- a/src/share/vm/runtime/globals.hpp  Fri Apr 04 10:04:44 2014 -0700
>>> +++ b/src/share/vm/runtime/globals.hpp  Wed Apr 09 22:05:18 2014 +0900
>>> @@ -2333,10 +2333,6 @@
>>>     develop(bool, TraceClassLoaderData,
>>> false,                                \
>>>             "Trace class loader loader_data
>>> lifetime")                        \
>>> \
>>> -  product(uintx,
>>> InitialBootClassLoaderMetaspaceSize,                       \
>>> -          NOT_LP64(2200*K)
>>> LP64_ONLY(4*M),                                  \
>>> -          "Initial size of the boot class loader data
>>> metaspace")           \
>>> - \
>>>     product(bool, TraceGen0Time,
>>> false,                                       \
>>>             "Trace accumulated time for Gen 0
>>> collection")                    \
>>> \
>>> ------------
>>>
>>> I prefer "B" .
>>> Because, I guess many users think MetaspaceSize decides initial
>>> metaspace size.
>>>
>>> If my idea is correct, I will file this to JBS and upload patch to
>>> webrev.
>> Please only correct the description of MetaspaceSize.
>>
>> Jon
>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>