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

[Jesper Wilhelmsson]

Looks good!
/Jesper

Alexander Harlap skrev den 19/8/15 23:02:
> 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
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>