[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-gc-dev
Subject:    Re: RFR (XS): 8025605: G1: Heap expansion logging misleading for fully expanded heap
From:       Bengt Rutisson <bengt.rutisson () oracle ! com>
Date:       2013-09-30 8:31:06
Message-ID: 524936CA.3090401 () oracle ! com
[Download RAW message or body]


Hi Thomas,

On 9/30/13 9:20 AM, Thomas Schatzl wrote:
> Hi,
>
> On Mon, 2013-09-30 at 09:01 +0200, Bengt Rutisson wrote:
>> Hi Thomas,
>>
>> Thanks for looking at this! Good comments. Here's an updated webrev:
>>
>> http://cr.openjdk.java.net/~brutisso/8025605/webrev.01/
>    the change looks good now, thanks.
>
> However, as Jesper mentioned, there is an issue with expansion that may
> make the change seemingly not work in many cases:
> VirtualSpace::expand_by() returns false if the number of bytes we want
> to expand is larger than the available number of bytes.
>
> I noticed that behavior in other PSR logs, but could not pinpoint it: g1
> did not expand the heap to maximum size, because, I believe now, this
> issue.
>
> I.e. the number of requested bytes to expand has been larger than the
> uncommitted available memory, so expand_by always returned false
> although there would be still enough space left to expand a little.
>
> However I think this should be fixed in an extra CR: I saw some code
> paths where heap expansion to the maximum heap size may not really
> change the situation (out of memory), e.g. when coming from
> expand_and_allocate().
>
> Other places, like the path from humongous_obj_allocate() does take the
> situation into account when expansion did not expand enough bytes, but
> others like the path from do_collection_pause_at_safepoint() would be
> fine.

Good point. I agree that this is something that needs to be fixed, but, 
as you say, it should probably be fixed as a separate change.

Thanks,
Bengt

>
> Thanks,
> Thomas
>
>
>

[prev in list] [next in list] [prev in thread] [next in thread]