[Nestmates] RFR: 8189163: [Nestmates] Updated invokeInterface selection/resolution rules

[David Holmes]

On 8/03/2018 7:22 PM, Tobias Hartmann wrote:
> 
> On 08.03.2018 09:53, David Holmes wrote:
>>> Just wondering, the change in dependencies.cpp wasn't necessary for functionality/correctness but is
>>> just an optimization, right?
>>
>> No it's necessary. Without it we get an assertion failure:
>>
>> #  Internal Error
>> (/export/users/dh198349/valhalla/repos/valhalla-exp/open/src/hotspot/share/code/dependencies.cpp:686),
>> pid=17815, tid=17868
>> #  assert(counter_changed) failed: failed dependencies, but counter didn't change
>>
>> My thinking was that the is_witness check found the private method and considered it an indicator of
>> a failed dependency (which it isn't), but no changes had occurred that should have invalidated the
>> dependencies, hence the assertion failed.
> 
> Yes, the assert indicates that an assumption (probably DepType::unique_concrete_method?) that did
> hold during compilation does not hold anymore after the compilation although the system dictionary
> modification counter didn't change.
> 
> But since the original assumption is also based on CHA (and should therefore also consider private
> methods), I wonder why that only fails after compilation.

I don't know how the original dependencies are determined, but if they 
utilize the itable then the private class method does not appear in the 
itable with these selection changes and so is not seen. But the 
is_witness check would see it. The two parts have to use the same basic 
logic for identifying candidate methods.

> Do you have a regression test for that? And if so, does this reproduce without nestmates?

The test that triggered this was:

compiler/jsr292/methodHandleExceptions/TestAMEnotNPE.java

and no it doesn't reproduce outside of nestmates.

Thanks,
David

> Thanks,
> Tobias
>