Quantcast

RFR: 8170455: C2: Access to [].clone from interfaces fails

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR: 8170455: C2: Access to [].clone from interfaces fails

Jamsheed C m
Hi,

Array type are treated as Object type in Compiler  (see [1]) and lookup
happens in Object Class in compiler interface. This used to work before
default method got introduced. with default methods protected Object
methods are inaccessible from interface , so lookup fails,   caller
method always deopt as unloaded and decompile in C2. This causes
considerable degradation in performance.

Fix: array types are directly passed to lookup code(fix), all other code
is kept as it is.

bug id: https://bugs.openjdk.java.net/browse/JDK-8170455

webrev:  http://cr.openjdk.java.net/~jcm/8170455/webrev.00/

Best Regards,

Jamsheed

[1]

ciInstanceKlass*
ciEnv::get_instance_klass_for_declared_method_holder(ciKlass*
method_holder) {
   // For the case of <array>.clone(), the method holder can be a
ciArrayKlass
   // instead of a ciInstanceKlass.  For that case simply pretend that the
   // declared holder is Object.clone since that's where the call will
bottom out.
   // A more correct fix would trickle out through many interfaces in CI,
   // requiring ciInstanceKlass* to become ciKlass* and many more places
would
   // require checks to make sure the expected type was found. Given
that this
   // only occurs for clone() the more extensive fix seems like overkill so
   // instead we simply smear the array type into Object.





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170455: C2: Access to [].clone from interfaces fails

Jamsheed C m
Hi,

Vladimir Ivanov suggested a few changes in webrev.00

1)  to move class accessibility check in lookup inside assert, as it is
already being done by caller once.

2)  to remove changes in resolve_invoke as it is not really required.

revised webrev: http://cr.openjdk.java.net/~jcm/8170455/webrev.01/

Thanks a lot for detailed review Vladimir.

Best Regards,

Jamsheed


On 1/31/2017 3:55 PM, Jamsheed C m wrote:

> Hi,
>
> Array type are treated as Object type in Compiler  (see [1]) and
> lookup happens in Object Class in compiler interface. This used to
> work before default method got introduced. with default methods
> protected Object methods are inaccessible from interface , so lookup
> fails,   caller method always deopt as unloaded and decompile in C2.
> This causes considerable degradation in performance.
>
> Fix: array types are directly passed to lookup code(fix), all other
> code is kept as it is.
>
> bug id: https://bugs.openjdk.java.net/browse/JDK-8170455
>
> webrev:  http://cr.openjdk.java.net/~jcm/8170455/webrev.00/
>
> Best Regards,
>
> Jamsheed
>
> [1]
>
> ciInstanceKlass*
> ciEnv::get_instance_klass_for_declared_method_holder(ciKlass*
> method_holder) {
>   // For the case of <array>.clone(), the method holder can be a
> ciArrayKlass
>   // instead of a ciInstanceKlass.  For that case simply pretend that the
>   // declared holder is Object.clone since that's where the call will
> bottom out.
>   // A more correct fix would trickle out through many interfaces in CI,
>   // requiring ciInstanceKlass* to become ciKlass* and many more
> places would
>   // require checks to make sure the expected type was found. Given
> that this
>   // only occurs for clone() the more extensive fix seems like
> overkill so
>   // instead we simply smear the array type into Object.
>
>
>
>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170455: C2: Access to [].clone from interfaces fails

Vladimir Ivanov
> revised webrev: http://cr.openjdk.java.net/~jcm/8170455/webrev.01/

Looks good.

Best regards,
Vladimir Ivanov

> On 1/31/2017 3:55 PM, Jamsheed C m wrote:
>> Hi,
>>
>> Array type are treated as Object type in Compiler  (see [1]) and
>> lookup happens in Object Class in compiler interface. This used to
>> work before default method got introduced. with default methods
>> protected Object methods are inaccessible from interface , so lookup
>> fails,   caller method always deopt as unloaded and decompile in C2.
>> This causes considerable degradation in performance.
>>
>> Fix: array types are directly passed to lookup code(fix), all other
>> code is kept as it is.
>>
>> bug id: https://bugs.openjdk.java.net/browse/JDK-8170455
>>
>> webrev:  http://cr.openjdk.java.net/~jcm/8170455/webrev.00/
>>
>> Best Regards,
>>
>> Jamsheed
>>
>> [1]
>>
>> ciInstanceKlass*
>> ciEnv::get_instance_klass_for_declared_method_holder(ciKlass*
>> method_holder) {
>>   // For the case of <array>.clone(), the method holder can be a
>> ciArrayKlass
>>   // instead of a ciInstanceKlass.  For that case simply pretend that the
>>   // declared holder is Object.clone since that's where the call will
>> bottom out.
>>   // A more correct fix would trickle out through many interfaces in CI,
>>   // requiring ciInstanceKlass* to become ciKlass* and many more
>> places would
>>   // require checks to make sure the expected type was found. Given
>> that this
>>   // only occurs for clone() the more extensive fix seems like
>> overkill so
>>   // instead we simply smear the array type into Object.
>>
>>
>>
>>
>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170455: C2: Access to [].clone from interfaces fails

Jamsheed C m
Thank you, Vladimir!

Best Regards,

Jamsheed

On 2/6/2017 7:22 PM, Vladimir Ivanov wrote:

>> revised webrev: http://cr.openjdk.java.net/~jcm/8170455/webrev.01/
>
> Looks good.
>
> Best regards,
> Vladimir Ivanov
>
>> On 1/31/2017 3:55 PM, Jamsheed C m wrote:
>>> Hi,
>>>
>>> Array type are treated as Object type in Compiler  (see [1]) and
>>> lookup happens in Object Class in compiler interface. This used to
>>> work before default method got introduced. with default methods
>>> protected Object methods are inaccessible from interface , so lookup
>>> fails,   caller method always deopt as unloaded and decompile in C2.
>>> This causes considerable degradation in performance.
>>>
>>> Fix: array types are directly passed to lookup code(fix), all other
>>> code is kept as it is.
>>>
>>> bug id: https://bugs.openjdk.java.net/browse/JDK-8170455
>>>
>>> webrev:  http://cr.openjdk.java.net/~jcm/8170455/webrev.00/
>>>
>>> Best Regards,
>>>
>>> Jamsheed
>>>
>>> [1]
>>>
>>> ciInstanceKlass*
>>> ciEnv::get_instance_klass_for_declared_method_holder(ciKlass*
>>> method_holder) {
>>>   // For the case of <array>.clone(), the method holder can be a
>>> ciArrayKlass
>>>   // instead of a ciInstanceKlass.  For that case simply pretend
>>> that the
>>>   // declared holder is Object.clone since that's where the call will
>>> bottom out.
>>>   // A more correct fix would trickle out through many interfaces in
>>> CI,
>>>   // requiring ciInstanceKlass* to become ciKlass* and many more
>>> places would
>>>   // require checks to make sure the expected type was found. Given
>>> that this
>>>   // only occurs for clone() the more extensive fix seems like
>>> overkill so
>>>   // instead we simply smear the array type into Object.
>>>
>>>
>>>
>>>
>>>
>>

Loading...