RFR(M) 8148481: Devirtualize Klass::vtable

[Mikael Gerdin]

Hi Coleen,

On 2016-02-01 16:41, Coleen Phillimore wrote:
>
>
> On 2/1/16 9:32 AM, Mikael Gerdin wrote:
>> Hi Coleen,
>>
>> On 2016-02-01 14:22, Coleen Phillimore wrote:
>>>
>>> Mikael,
>>> This looks good also.
>>>
>>> http://cr.openjdk.java.net/~mgerdin/8148481/webrev.0/src/share/vm/oops/instanceKlass.cpp.udiff.html
>>>
>>>
>>>
>>> Why didn't you move print_vtable to klass.cpp too rather than casting to
>>> intptr_t?  That looks odd.
>>
>> Well, what I actually did was overload print_vtable with a variant
>> that casts the parameter to intptr_t*.
>> The reason it can't take a vtableEntry* is that print_vtable is called
>> for printing itables as well, and they have a more complex layout.
>
> Ah, I see.
>>
>> I'm not entirely happy with the solution but I'd rather not have
>> start_of_vtable return an intptr_t* just to avoid casting when dumping
>> some debug output, and doing a cast in the print line felt ugly for
>> some reason.
>
> Agree.
>>
>> print_vtable should probably be named maybe_dump_metadata since it
>> actually dumps memory contents and if it's metadata it tries to call
>> the virtual print function on the metadata.
>>
> I don't know but you should leave it.  I like when printing functions
> start with print.

Ok, in this case I agree that the name should stay.

Thanks for the review!
/Mikael

>
> Coleen
>
>> /Mikael
>>
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 1/28/16 11:13 AM, Mikael Gerdin wrote:
>>>> Hi all,
>>>>
>>>> Due to my recent changes in this area, Klass is now responsible for
>>>> maintaining the offsets and length of the embedded vtable and
>>>> therefore it makes sense to move all code related to the Java vtables
>>>> to Klass.
>>>>
>>>> This also allows us to remove a few unsafe casts where an ArrayKlass
>>>> was cast to an InstanceKlass just to get at the methods_at_vtable().
>>>> These casts were removed from reflection.cpp, jni.cpp,
>>>> jvmciCompilerToVM.cpp and linkResolver.cpp, in cpCache.cpp there was
>>>> an alternate approach to the same problem which I've rewritten to use
>>>> the new way of accessing the vtable directly through Klass.
>>>>
>>>> Some notes:
>>>> * I took the liberty of changing the return type of start_of_vtable()
>>>> to vtableEntry* since that is in fact what it is.
>>>> * method_at_vtable is no longer inline, but I don't think that should
>>>> be a performance problem since it's usually only being called from
>>>> link resolving, cpCache or JNI calls, all of which are not
>>>> particularly hot paths.
>>>>
>>>>
>>>> Bug link: https://bugs.openjdk.java.net/browse/JDK-8148481
>>>> Webrev: http://cr.openjdk.java.net/~mgerdin/8148481/webrev.0
>>>>
>>>> Testing: JPRT + RBT on Oracle platforms.
>>>>
>>>> /Mikael
>>>
>>
>