Pls review 6887571

[Y.S.Ramakrishna at Sun.COM]

On 10/28/09 12:25, Paul Hohensee wrote:
> Thanks for the review (and the previous one too).
> 
> Yes, I feel queasy about changing CMS ergo.  Using the code in
> set_heap_size() with cms produced some pretty weird results, such
> as initial gen1 sizes of 64kb.  I investigated a bit and decided to leave
> well enough alone.  I'll be happy to file an rfe to integrate cms and
> common heap ergo.  Would that be acceptable?

Absolutely. Thanks for filing the rfe!

-- ramki

> 
> Paul
> 
> Y.S.Ramakrishna at Sun.COM wrote:
>> On 10/27/09 15:12, Paul Hohensee wrote:
>>> Yes, I forgot to uncomment line 1383.  Fixed.
>>>
>>> Fixed line 1612.
>>
>> Thanks.
>>
>>>
>>> It's a good thing you asked me to get cms dumps.  Turned out I'd 
>>> typo'ed and
>>> had an earlier version of arguments.cpp.  The earlier version didn't 
>>> separate
>>> cms ergo from everything-else ergo.  New webrev here
>>>
>>> http://cr.openjdk.java.net/~phh/6887571/webrev.02/
>>
>> I think I will need to discuss this further with you. I actually prefer
>> the older version you had where the heap size is set ergonomically unless
>> explicitly specified in the case of CMS as well. The CMS ergo will take
>> that as a given and _shape_ the generations within those constraints. 
>> That would be
>> my preferred model of CMS ergo -- basically, what you had in your
>> earlier webrev. However, I understand if you and others feel leery about
>> touching CMS ergo for other reasons and would prefer any such changes 
>> to be
>> affected in a subsequent CR which does due diligence in
>> measuring the performance delta of that change for CMS specifically.
>>
>> -- ramki
>>
>>>
>>> With the new version, running
>>>
>>> java -client -XX:+UseConcMarkSweepGC -XX:+PrintGCDetails -XX:+Verbose 
>>> -version
>>>
>>> I get
>>>
>>> VM option '+UseConcMarkSweepGC'
>>> VM option '+PrintGCDetails'
>>> VM option '+Verbose'
>>> Turning off use of shared archive because of choice of garbage 
>>> collector or large pages
>>> Ergo set MaxNewSize: 134217728
>>> CMS set min_heap_size: 0 initial_heap_size:  0 max_heap: 100663296
>>> Ergo set NewSize: 16777216
>>> Ergo set OldSize: 50331648
>>> Minimum heap 67108864  Initial heap 67108864  Maximum heap 100663296
>>> Minimum gen0 16777216  Initial gen0 16777216  Maximum gen0 16777216
>>> Minimum gen1 50331648  Initial gen1 50331648  Maximum gen1 83886080
>>>
>>> java version "1.7.0-ea"
>>> Java(TM) SE Runtime Environment (build 1.7.0-ea-b39)
>>> OpenJDK Client VM (build 17.0-b03-internal-jvmg, mixed mode)
>>> Heap
>>> par new generation   total 14784K, used 527K [0xf1800000, 0xf2800000, 
>>> 0xf2800000)
>>>  eden space 13184K,   4% used [0xf1800000, 0xf1883de8, 0xf24e0000)
>>>  from space 1600K,   0% used [0xf24e0000, 0xf24e0000, 0xf2670000)
>>>  to   space 1600K,   0% used [0xf2670000, 0xf2670000, 0xf2800000)
>>> concurrent mark-sweep generation total 49152K, used 0K [0xf2800000, 
>>> 0xf5800000, 0xf7800000)
>>> concurrent-mark-sweep perm gen total 12288K, used 1497K [0xf7800000, 
>>> 0xf8400000, 0xfb800000)
>>>
>>> With the unmodified vm, I get
>>>
>>> VM option '+UseConcMarkSweepGC'
>>> VM option '+PrintGCDetails'
>>> VM option '+Verbose'
>>> Turning off use of shared archive because of choice of garbage 
>>> collector or large pages
>>> Ergo set MaxNewSize: 134217728
>>> CMS set min_heap_size: 0 initial_heap_size:  0 max_heap: 67108864
>>> Ergo set NewSize: 16777216
>>> Ergo set OldSize: 50331648
>>> Minimum heap 67108864  Initial heap 67108864  Maximum heap 67108864
>>> Minimum gen0 16777216  Initial gen0 16777216  Maximum gen0 16777216
>>> Minimum gen1 50331648  Initial gen1 50331648  Maximum gen1 50331648
>>>
>>> java version "1.7.0-ea"
>>> Java(TM) SE Runtime Environment (build 1.7.0-ea-b39)
>>> OpenJDK Client VM (build 17.0-b03-internal-jvmg, mixed mode)
>>> Heap
>>> par new generation   total 14784K, used 527K [0xf3800000, 0xf4800000, 
>>> 0xf4800000)
>>>  eden space 13184K,   4% used [0xf3800000, 0xf3883de8, 0xf44e0000)
>>>  from space 1600K,   0% used [0xf44e0000, 0xf44e0000, 0xf4670000)
>>>  to   space 1600K,   0% used [0xf4670000, 0xf4670000, 0xf4800000)
>>> concurrent mark-sweep generation total 49152K, used 0K [0xf4800000, 
>>> 0xf7800000, 0xf7800000)
>>> concurrent-mark-sweep perm gen total 12288K, used 1497K [0xf7800000, 
>>> 0xf8400000, 0xfb800000)
>>>
>>> They're identical, except for the total heap size and ergo-computed 
>>> old gen
>>> size, which are 96m and 80m respectively.  That's because I increased 
>>> the
>>> value of MaxNewSize from 64m to 96m.
>>>
>>> -server instead of -client produces the same result.
>>>
>>> Paul
>>>
>>> Y.S.Ramakrishna at Sun.COM wrote:
>>>> Hi Paul --
>>>>
>>>> Did you forget to remove the commenting out of the second conjunct in
>>>> arguments.cpp:1383 below:-
>>>>
>>>>   1383     if (PrintGCDetails/* && Verbose*/) {
>>> Yes, fixed.
>>>>
>>>> Could you do me a favour and change the following in 
>>>> arguments.cpp:1612 from:-
>>>>
>>>>   1612       if (UseSharedSpaces) {
>>>>
>>>> to:-
>>>>
>>>>   1612       if (UseSharedSpaces && Verbose) {
>>>>
>>>> The verbosity of the message at 1613:1615 has been
>>>> bothering some of us for a while (mea culpa for adding
>>>> it recentkly), but i didn't get around to removing it, and this
>>>> looks like an good opportunity to do so with no new overhead :-)
>>> Done.
>>>>
>>>> On 10/27/09 09:26, Paul Hohensee wrote:
>>>> ...
>>>>> 6. In arguments.cpp, set_heap_size() replaces 
>>>>> set_server_heap_size() and is used for everything
>>>>> except CMS.  CMS has it's own, incompatible (I know: I tried it), 
>>>>> heap sizing ergonomics.
>>>>
>>>> Could you provide the result of:-
>>>>
>>>> % java -XX:+UseConcMarkSweepGC -XX:+PrintGCDetails -version
>>>>
>>>> with the baseline VM and with the VM after yr changes?
>>>>
>>>>
>>>> Rest looks good to me; thanks!
>>>> -- ramki
>>>>
>>