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

[Jesper Wilhelmsson]

Looks good!
/Jesper

Den 11/9/15 kl. 19:53, skrev 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
>>
>>
>