Review request (S): 7131791 G1: Asserts in nightly testing due to 6976060

[Tony Printezis]

Stefan,

It's a fundamental assumption of G1 that we never move humongous objects 
during a GC. In the future, we might consider them for collection during 
a GC, however they will either be reclaimed or be left where they are. 
Given that a humongous object takes up whole regions, there's no much 
point in moving it somewhere else.

Tony

On 01/20/2012 11:53 AM, Stefan Karlsson wrote:
> On 2012-01-20 17:56, Bengt Rutisson wrote:
>>
>> Stefan, are you OK with adding the assert that Tony suggested?
>
> Use the assert if you want to, but I don't see why that would be a 
> better solution. If we ever start moving humongous objects, returning 
> h() will work in production code, while the assert will only be found 
> in debug builds.
>
> StefanK
>
>>
>> Bengt
>>
>> On 2012-01-20 17:44, Stefan Karlsson wrote:
>>> On 2012-01-20 17:46, Bengt Rutisson wrote:
>>>>
>>>> Stefan,
>>>>
>>>> Thanks for the prompt review!
>>>>
>>>> Comments inline.
>>>>
>>>> On 2012-01-20 17:21, Stefan Karlsson wrote:
>>>>> On 20 jan 2012, at 17:06, Bengt Rutisson 
>>>>> <bengt.rutisson at oracle.com <mailto:bengt.rutisson at oracle.com>> wrote:
>>>>>
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Can I have a couple of quick reviews for this small change:
>>>>>> http://cr.openjdk.java.net/~brutisso/7131791/webrev.02 
>>>>>> <http://cr.openjdk.java.net/%7Ebrutisso/7131791/webrev.02>
>>>>>>
>>>>>> This should hopefully fix the 500+ failures in the G1 nightlies. 
>>>>>> So, I would like to get it in before the nightlies tonight.
>>>>>>
>>>>>> The issue is that we call collect() which will trigger a 
>>>>>> collection without protecting the memory that we just allocated 
>>>>>> for a humongous object. The fix (thanks Tony for helping me 
>>>>>> out!!!) is to fake an object and create a handle to it before we 
>>>>>> call collect.
>>>>>
>>>>> 1067         Handle h((oop)result);  
>>>>> 1068         collect(GCCause::_g1_humongous_allocation);
>>>>> 1069       }
>>>>> 1070       return result;
>>>>> 1071     }
>>>>> Can we really have a handle to uninitialized memory?
>>>>
>>>> The memory is not uninitialized since I fake an object there with 
>>>> the call to CollectedHeap::fill_with_object(result, word_size, 
>>>> false); just before the code you have above.
>>>
>>> I missed that.
>>>
>>>>
>>>>> Are you sure that the humongous object will not be moved by a full collection. You should probably return h() instead of result.
>>>>
>>>> Humongous objects will not be moved by G1 collections so I think we 
>>>> are ok.
>>>
>>> OK. But maybe we should be a bit defensive and return h() here.
>>>
>>> StefanK
>>>
>>>>
>>>> Thanks for the prompt review!
>>>>
>>>> Bengt
>>>>>
>>>>> StefanK
>>>>>
>>>>>>
>>>>>> Bengt
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20120120/64baf537/attachment.htm>