Request for review(S): 7003554: (tiered) assert(is_null_object() || handle() != NULL) failed: cannot embed null pointer

[Igor Veresov]

Done.

Webrev: http://cr.openjdk.java.net/~iveresov/7003554/webrev.02/

igor

On 12/1/10 9:44 PM, Tom Rodriguez wrote:
>
> On Dec 1, 2010, at 9:07 PM, Igor Veresov wrote:
>
>> On 12/1/10 8:22 PM, Tom Rodriguez wrote:
>>>
>>> On Dec 1, 2010, at 5:42 PM, Igor Veresov wrote:
>>>
>>>> C1 with profiling doesn't check whether the MDO has been really allocated, which can silently fail if the perm gen is full. The solution is to check if the allocation failed and bailout out of inlining or compilation.
>>>
>>> The logic around build_method_data doesn't look right to me.  Won't it return false if there is already a valid MDO which would cause us to incorrectly bailout?
>>
>> Oops :( Right, fixed.
>>
>>> Maybe build_method_data should become ensure_method_data and return true if the method has a real MDO, creating it if necessary.
>>
>> But that's kind of what it does, except for accessors, etc...? Would it be just the name change?
>
> Yes it would be just a name change after your fix.  Having a boolean return from something called build_method_data seems to imply that it should work the way it did in your original webrev, return true if it created something.  The name change seems like it would match the return value better, though I don't feel that strongly about it.
>
>>
>>>
>>> Shouldn't this assert:
>>>
>>> +     assert(md != NULL, "Sanity");
>>>
>>> be stronger?  It seems like it should be checking that it's not an empty method data too since those won't have a real object behind them.  Maybe C1 should be using a new method like method_data_or_null() so that it never sees empty method datas.
>>>
>>
>> Good idea, done.
>
> Could you put a short comment on method_data_or_null.  Otherwise it looks good.
>
> tom
>
>>
>>
>> Updated webrev: http://cr.openjdk.java.net/~iveresov/7003554/webrev.01/
>>
>> igor
>>
>>> tom
>>>
>>>>
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~iveresov/7003554/webrev.00/
>>>>
>>>>
>>>> Tested with the failing nightly test, specjvm98, and JPRT.
>>>>
>>>> Thanks,
>>>> igor
>>>
>>
>