Request for code review - 8130265: gctests/LargeObjects/large001 fails with OutOfMemoryError: Java heap space

[Alexander Harlap]

Here is new revision where fixed formatting:

http://cr.openjdk.java.net/~aharlap/8130265/webrev.06/

Alex

On 9/1/2015 4:12 AM, Thomas Schatzl wrote:
> Hi Alex,
>
> On Mon, 2015-08-31 at 11:38 -0400, Alexander Harlap wrote:
>> On 8/31/2015 4:40 AM, Thomas Schatzl wrote:
>>> Hi,
> [...]
>>>>>>>> I also have a question about the order of
>>>>>>>> attempt_allocation_at_safepoint() and the expand_and_allocate(), and
>>>>>>>> could not find this answered before: so full gc might shrink the heap
>>>>>>>> too much after full gc, and we should expand the heap.
>>>>>>>> If I were to do this change, there would always (unconditionally) be an
>>>>>>>> expansion after full gc, because the current change only expands if the
>>>>>>>> allocation after gc fails. Also the 02 change.
>>>>>>>>
>>>>>>>> The review thread so far suggests that full gc might shrink too much,
>>>>>>>> but this still seems to be the case here in some cases.
>>>>> Could you clarify this question? To me, this looks like a potential bug
>>>>> that has been overlooked so far in this change.
>>>> Yes, full GC in some situation may over-shrink heap such way that heap
>>>> expansion will be needed. It was reason for
>>>> gctests/LargeObjects/large001 failure.  Still I prefer to do expansion
>>>> only after allocation attempt failed. Why should we do unconditional
>>>> expansion ( even if it is not needed )?
>>> To keep the Min/MaxHeapFreeRatio constraints the heap should observe.
>>>
>>>>    In the existing code we have
>>>> call to expand_and_allocate() only after
>>>> attempt_allocation_at_safepoint() failure. What potential bug do you mean?
>>> That the Min/MaxHeapFreeRatio values are not observed, causing
>>> performance degradation, in the worst case like in this situation.
>>>
>>> The descriptions of the flags do not indicate an exception like "do not
>>> care if the GC thinks it is not required".
>>>
>>> Thanks,
>>>     Thomas
>>>
>> Min/MaxHeapFreeRatio already were observed in calculation of shrink value. I do
>> not see reason for expanding heap immediately after do_collection before doing
>> attempt to allocate.
> Okay, thanks for clarification, that has been the information I was
> missing.
>
> Thanks,
>    Thomas
>
>