RFR(S): 8069263: assert(fm == NULL || fm->method_holder() == _participants[n]) failed: sanity

[Roland Westrelin]

> Fix looks correct. I CC to runtime group. They may know answers for your questions.

Thanks for the review. We’ve been the ones making changes to dependencies.cpp for default methods, not sure the runtime team can help.

I tried removing:

  // Disable CHA for default methods for now
  if (root_m->get_Method()->is_default_method()) {
    return NULL;
  }

and the test case that comes with that change crashes. So it looks like it’s still needed.

Roland.

> 
> Thanks,
> Vladimir
> 
> On 3/12/15 6:31 AM, Roland Westrelin wrote:
>> http://cr.openjdk.java.net/~roland/8069263/webrev.00/
>> 
>> In the test case, I1.m1() is a default method that is added by default methods processing to the list of default methods of class C2. The call at line 54 triggers CHA. It finds m1 in class C2 but with holder I1 as unique target. The assert fires because m1’s holder is not C2 but m1 is in the list of C2’s methods.
>> 
>> I fixed this by returning no unique method in that case because:
>> 
>> 1) we seem to still not expect that we can handle default methods properly in CHA even though we’ve added code in dependencies.cpp to support them. ciMethod::find_monomorphic_target) has this code:
>> 
>>   // Disable CHA for default methods for now
>>   if (root_m->get_Method()->is_default_method()) {
>>     return NULL;
>>   }
>> 
>> Or maybe that code should be removed?
>> 
>> 2) a proper fix would require that we record the method, its actual holder (C2) in the dependency which would require more extensive changes. I wonder if the extra complexity would make sense.
>> 
>> Roland.
>> 
>>