-XX:MetaspaceSize is correct?

[Yasumasa Suenaga]

Jon,

I agree with you.

MetaspaceSize is used as minimum value when JVM shrink capacity in
MetaspaceGC::compute_new_size() .
So I will change description to "minimum" and upload new webrev.


Yasumasa

2014-04-15 7:58 GMT+09:00, Jon Masamitsu <jon.masamitsu at oracle.com>:
> Yasumasa,
>
> It's hard to describe what this parameter is in a single
> sentence.  "high-water-mark" doesn't mean much unless
> you know about how it is used in the GC.   Although
> "initial" is not right in the description, "minimum" might
> be closer to a good description.  I'm really not sure.
>
> Jon
>
> On 04/10/2014 05:20 AM, Yasumasa Suenaga wrote:
>> 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
>>>>>
>>>>
>>
>
>