[9] RFR (S) 8146436: Add -XX:+UseAggressiveHeapShrink option

[Tom Benson]

Hi Chris,
OK, that all sounds good.

 >> I can change it, although that will mean filing a new CCC.
Ah, I'd forgotten about that.  Not worth it, unless there's a landslide 
of support for a different name.

Tnx,
Tom

On 2/10/2016 3:06 PM, Chris Plummer wrote:
> Hi Tom,
>
> Thanks for having a look. Comments inline below:
>
> On 2/10/16 11:27 AM, Tom Benson wrote:
>> Hi Chris,
>> My apologies if I missed the discussion somewhere, but is there a 
>> specific rationale for adding this that can be mentioned in the bug 
>> report?  I can imagine scenarios where it would be useful, but maybe 
>> the real need can be called out.
> In general, it is for customers that want to minimize the amount of 
> memory used by the java heap, and are willing to sacrifice some 
> performance (induce more frequent GCs) to save that memory. When heap 
> usage fluctuates greatly, the GC will tend to hold on to that memory 
> longer than needed due to the the current algorithm which requires 4 
> full GCs before MaxHeapFreeRatio is fully honored. If this is what you 
> are looking for, I can add it to the CR.
>>
>> I think it might be clearer if the new code in cardGeneration was 
>> moved down to where the values are used.  IE, I would leave the inits 
>> of current_shrink_factor and _shrink_factor as they were at lines 
>> 190/191.   Then down at 270, just don't divide by the shrink factor 
>> if UseAggressiveHeapShrink is set, and the updates to shrink factor 
>> can be in the same conditional.  This has the advantage that you can 
>> fix the comment just above it to match this special case. Do you 
>> think that would work?
> Yes, that makes sense. I'll get started on it. I have a vacation 
> coming up shortly, so what I'll get a new webrev out soon, but 
> probably will need to wait until after my trip to do more thorough 
> testing and push the changes.
>>
>> It looks like the ending "\" at line 3330 in globals.hpp isn't 
>> aligned, and the copyright in cardGeneration.cpp needs to be updated.
> Ok.
>>
>> One other nit, which you can ignore unless someone comes forward to 
>> agree with me  8^)  , is that I'd prefer the name 
>> ShrinkHeapAggressively instead.   Maybe this was already debated 
>> elsewhere....
> The name choice hasn't really been discussed or questioned. It was 
> what was suggested to me, so I stuck with it (The initial work was 
> done by someone else. I'm just getting it integrated into 9). I can 
> change it, although that will mean filing a new CCC.
>
> thanks,
>
> Chris
>> Tom
>>
>> On 2/4/2016 1:36 PM, Chris Plummer wrote:
>>> Hello,
>>>
>>> Please review the following for adding the -XX 
>>> UseAggressiveHeapShrink option. When turned on, it tells the GC to 
>>> reduce the heap size to the new target size immediately after a full 
>>> GC rather than doing it progressively over 4 GCs.
>>>
>>> Webrev: http://cr.openjdk.java.net/~cjplummer/8146436/webrev.02/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8146436
>>>
>>> Testing:
>>>   -JPRT with '-testset hotspot'
>>>   -JPRT with '-testset hotspot -vmflags "-XX:+UseAggressiveHeapShrink"'
>>>   -added new TestMaxMinHeapFreeRatioFlags.java test
>>>
>>> thanks,
>>>
>>> Chris
>>
>