Free ratio based heap shrinking in the parallel collector

[Jon Masamitsu]

Hiroshi,

Looks fine.  I only looked at the files where you indicated a change in your
mail below.  If there is more for me to look at, please let me know 
which files.
I'll get another one of the GC guys here to also review
the changes. 

I've create a CR for this.

6952079: With UseParallelScavenge use MinHeapFreeRation/MaxHeapFreeRatio 
if UseAdaptiveSivePolicy is off.

What testing have you done?  I would expect that UseParallelGC with
UseAdaptiveSizePolicy turned off to grow and shrink the generations
in a way similar to UseSerialGC.  Can you check that is the case?
I think if you set SurvivorRatio explicitly the survivor spaces will
be close.  You might have to set InitialSurvivorRatio.   If you
set InitialTenuringThreshold and MaxTenuringThreshold  to the
same value, I would expect the promotion rate to be close.
Promotion rates don't have to be the same but it would remove
one variable from a comparison.  If there are surprises let talk about
them.  Different isn't necessarily wrong.

Jon

On 05/10/10 16:50, Hiroshi Yamauchi wrote:
> Jon,
>
> Thanks for your comments. My new revision is here
>
>   http://cr.openjdk.java.net/~hiroshi/webrevs/heapshrink/
>
> My comments are inlined below:
>
>   
>> In parallelScavengeHeap.cpp
>>
>> In the UseParallelGC collector the initial size (init_size) can be
>> significantly larger than the minimum size.   If you want to shrink down
>> the heap to minimize footprint, do you really want to limit by the
>> initial size instead of the minimum size?
>>
>> 1001     maximum_desired_capacity = MAX2(maximum_desired_capacity, init_size);
>>
>> Similarly here the amount of used data could be very small and
>> setting the floor at the initial size may not be what you
>> want.
>>
>> 1034     minimum_desired_capacity = MAX2(minimum_desired_capacity, init_size);
>>     
>
> I changed the code so that it uses the min size instead of the init size.
>
>   
>> In psYoungGen.cpp you guard output with PrintGC only.  In similar
>> cases this type of output is guarded with Verbose also.  Does this
>> output as is get printed in the middle of the usual
>> -verbosegc (also known as PrintGC) line?
>>
>> 1000   if (PrintGC) {
>> 1001     gclog_or_tty->print_cr(" Resizing young gen. expand_bytes=%d,%d",
>> 1002                            eden_expand_bytes, survivor_expand_bytes);
>> 1003     gclog_or_tty->print("BEFORE: Young Gen: ");
>> 1004     gclog_or_tty->print("eden capacity : " SIZE_FORMAT ", ",
>> 1005                         eden_space()->capacity_in_bytes());
>> 1006     gclog_or_tty->print("eden used : " SIZE_FORMAT ", " ,
>> 1007                         eden_space()->used_in_bytes());
>> 1008     gclog_or_tty->print("survivor capacity : " SIZE_FORMAT ", ",
>> 1009                         from_space()->capacity_in_bytes());
>> 1010     gclog_or_tty->print_cr("survivor used : " SIZE_FORMAT ", " ,
>> 1011                            from_space()->used_in_bytes());
>> 1012   }
>>     
>
> I added Verbose to the guard. And yes, the output show up in the
> middle of PrintGC line.
>
>   
>> In psOldGen.cpp you could try to expand up to max_gen_size
>> instead of returning?
>>
>>  501 void PSOldGen::try_to_expand_by(size_t expand_bytes) {
>>  502   if (expand_bytes < MinHeapDeltaBytes ||
>>  503       capacity_in_bytes() + expand_bytes > max_gen_size()) {
>>  504     return;
>>
>> Additionally PSOldGen::expand() chooses to use MinHeapDeltaBytes
>> as the minimum expansion size (in the sense that sizes for expansion
>> are round up to MinHeapDeltaBytes).   You would rather not expand
>> for less than MinHeapDeltaBytes?   I'll admit that there maybe some
>> inconsistencies in the way MinHeapDeltaBytes is used.  Just checking
>> that this is what you want to do.
>>
>> Again, seems to me that you want to shrink down to the minimum
>> generation size as opposed to the initial size.
>>
>>  523 void PSOldGen::try_to_shrink_by(size_t shrink_bytes) {
>>  524   if (shrink_bytes < MinHeapDeltaBytes ||
>>  525       capacity_in_bytes() - shrink_bytes < init_gen_size()) {
>>  526     return;
>>  527   }
>>     
>
> Now I have:
>
>  502   if (capacity_in_bytes() + expand_bytes > max_gen_size()) {
>  503     expand_bytes = max_gen_size() - capacity_in_bytes();
>  504   }
>
> and likewise for try_to_shrink_by().
>
> 1. It allows the subsequent code (eg expand()) to round up the size <
> MinHeapDeltaBytes.
> 2. It rounds down the size that goes past the max/min gen size so that
> the heap is expanded/shrunk to the max/min.
>
>   
>> In globals.hpp.  We may want this feature implemented in other
>> collectors at some point - G1 comes to mind.  I'd drop the leading
>> PS on the flag so that it is ResizeByFreeRatioWithSystemGC.
>>
>> 3079   product(bool, PSResizeByFreeRatioWithSystemGC, false,                     \
>> 3080           "Resize the heap by free ratio in System.gc() "                   \
>> 3081           "under UseParallelGC")
>>
>>     
>
> Done.
>
> Hiroshi
>   
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20100513/61b8abaa/attachment.htm>