RFR (XS): 8025605: G1: Heap expansion logging misleading for fully expanded heap

[Bengt Rutisson]

Hi Thomas,

Thanks for looking at this! Good comments. Here's an updated webrev:

http://cr.openjdk.java.net/~brutisso/8025605/webrev.01/

Bengt

On 9/28/13 9:45 AM, Thomas Schatzl wrote:
> Hi Bengt,
>
> On Fri, 2013-09-27 at 21:57 +0200, Bengt Rutisson wrote:
>> Hi everyone,
>>
>> Could I have a couple of reviews for this small change?
>>
>> http://cr.openjdk.java.net/~brutisso/8025605/webrev.00/
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8025605
>>
>> Background:
>>
>> When running G1 with -XX:+PrintAdaptiveSizePolicy some information
>> about the heap sizing is logged. When the heap is fully expanded but
>> the ergonomics would have liked to expand even more the following
>> message is logged:
>>
>> [G1Ergonomics (Heap Sizing) did not expand the heap, reason: heap
>> expansion operation failed]
>>
>> It sounds like something failed, but in reality we just hit the -Xmx
>> value. We should have a more informative message that does not look
>> like a serious failure.
>>
>> With my suggested patch the message will instead be:
>>
>> [G1Ergonomics (Heap Sizing) did not expand the heap, reason: heap
>> already fully expanded]
>>
> I would somewhat actually prefer if the message were moved after the
> message printed in line 1787. Otherwise this message would be different
> to others if verbosity is high enough. I.e. missing the "expand the
> heap" message with the two sizes message.
>
> Also, could you remove the assignment to the local variable
> old_mem_size? I think the variable is unused. (As a bonus you could
> remove the same assignment to a local "old_mem_size" variable in
> shrink_helper too :)
>
> Thanks,
>    Thomas
>
>
>