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

[Jesper Wilhelmsson]

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
>>>>>>>
>>>>>
>>>
>