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

[Alexander Harlap]

I agree.

Here is new revision:

http://cr.openjdk.java.net/~aharlap/8130265/webrev.02/
<http://cr.openjdk.java.net/%7Eaharlap/8130265/webrev.02/>
Alex

On 8/19/2015 4:36 PM, Jesper Wilhelmsson wrote:
> Hi Alex,
>
> Yes, I like this version much better.
> I'm fine with the change as is, but would it make sense to add the 
> same code after the first GC as well? So that every 
> attempt_allocation_at_safepoint is followed by an expand_and_allocate. 
> We could avoid a GC in some cases.
> /Jesper
>
> Alexander Harlap skrev den 19/8/15 22:14:
>> Hi Jesper,
>>
>> I agree that touching code  in 
>> resize_if_necessary_after_full_collection(size_t
>> word_size) is not pretty.
>>
>> Instead of doing  precise shrink, let it "over shrink" and expand after.
>>
>> Here is link to new version of change:
>>
>> http://cr.openjdk.java.net/~aharlap/8130265/webrev.01/
>>
>> Alex
>>
>> On 8/18/2015 4:25 PM, Jesper Wilhelmsson wrote:
>>> Hi Alex,
>>>
>>> Alexander Harlap skrev den 18/8/15 21:51:
>>>> Hi Jesper,
>>>>
>>>> I need whole HeapRegion(s) in aligned_bytes_to_allocate,
>>>> so operations on doubles will result in rounding and will not work.
>>>> If you care about overflowing size_t by doing += 
>>>> aligned_bytes_to_allocate, it
>>>> can avoided with code like this:
>>>>
>>>> minimum_desired_capacity =  ((max_heap_size - 
>>>> minimum_desired_capacity) >
>>>> aligned_bytes_to_allocate) ? minimum_desired_capacity +
>>>> aligned_bytes_to_allocate : max_heap_size ;
>>>>
>>>> maximum_desired_capacity =  ((max_heap_size - 
>>>> maximum_desired_capacity) >
>>>> aligned_bytes_to_allocate) ? maximum_desired_capacity +
>>>> aligned_bytes_to_allocate : max_heap_size ;
>>>
>>> Yes, I think we need to do something like this. It seems odd to go 
>>> through all
>>> that trouble above to avoid overflowing and then just add a random size
>>> afterwards.
>>>
>>> I never really formulated my original concern though. I may be wrong 
>>> here so
>>> feel free to interrupt at any point. The min and max values 
>>> represent a range
>>> of sizes that we consider desirable. By adding the new size to both 
>>> min and
>>> max we move that entire range. That seems wrong to me. If the size 
>>> we need is
>>> already below the max value, all we need to do is to add to the min 
>>> value. Or
>>> maybe we need to add a little to max, but not the entire size. Or is 
>>> it the
>>> case that the max value will always be too small?
>>>
>>> Thanks,
>>> /Jesper
>>>
>>>>
>>>>
>>>> On 8/18/2015 1:39 PM, Jesper Wilhelmsson wrote:
>>>>> Alexander Harlap skrev den 18/8/15 18:36:
>>>>>> Hi Jesper,
>>>>>>
>>>>>> Here minimum_desired_capacity and maximum_desired_capacity are 
>>>>>> calculated to
>>>>>> accommodate already allocated sstuff,
>>>>>> aligned_bytes_to_allocate - extra size needed to allocate new 
>>>>>> memory (gc was
>>>>>> called when we failed to allocate this new memory).
>>>>>
>>>>> Right. Would it make sense to put this new addition around line 
>>>>> 1658 instead,
>>>>> before we start to adjust the values with respect to max_heap_size?
>>>>> /Jesper
>>>>>
>>>>>>
>>>>>> Alex
>>>>>>
>>>>>>
>>>>>> On 8/18/2015 12:15 PM, Jesper Wilhelmsson wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> +  minimum_desired_capacity += aligned_bytes_to_allocate;
>>>>>>> +  maximum_desired_capacity += aligned_bytes_to_allocate;
>>>>>>>
>>>>>>> Is it desired to always increase the desired capacity rather 
>>>>>>> than doing it
>>>>>>> only if it is too small? E.g.:
>>>>>>>
>>>>>>> minimum_desired_capacity = MAX2(minimum_desired_capacity,
>>>>>>> aligned_bytes_to_allocate);
>>>>>>> maximum_desired_capacity = MAX2(maximum_desired_capacity,
>>>>>>> aligned_bytes_to_allocate);
>>>>>>>
>>>>>>> Thanks,
>>>>>>> /Jesper
>>>>>>>
>>>>>>>
>>>>>>> Alexander Harlap skrev den 18/8/15 18:02:
>>>>>>>> This bug was caused by  too aggressive heap shrinkage in G1.
>>>>>>>>
>>>>>>>> JDK-8130265 <https://bugs.openjdk.java.net/browse/JDK-8130265>
>>>>>>>>
>>>>>>>> Proposed fix:
>>>>>>>> http://cr.openjdk.java.net/~aharlap/8130265/webrev.00
>>>>>>>> <http://cr.openjdk.java.net/%7Eaharlap/8130265/webrev.00>
>>>>>>>> Testing: JPRT, tonga
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Alex
>>>>>>>>
>>>>>>
>>>>
>>