-XX:MetaspaceSize is correct?

[Yasumasa Suenaga]

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

Please review and sponsoring.


Yasumasa

On 04/15/2014 12:48 PM, Yasumasa Suenaga wrote:
> 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
>>>>>>
>>>>>
>>>
>>
>>