RFR: 8253049: Enhance itable_stub for AArch64 and x86_64

[Vladimir Ivanov]

Hi Kevin,

Very interesting observations. I like the idea to optimize for the case 
when REFC == DECC.

Fusing 2 passes over the itable into one does look attractive, but I'm 
not sure the proposed variant is correct. I suggest to split the patch 
into 2 enhancements and handle them separately.

I'm curious what kind of benchmarks you used and what are the 
improvements observed with the patch.

One suggestion about the implementation:

src/hotspot/cpu/x86/macroAssembler_x86.cpp:

+void MacroAssembler::lookup_interface_method_in_stub(Register recv_klass,

I'd like to avoid having 2 independent implementations of itable lookup 
(MacroAssembler::lookup_interface_method_in_stub() and 
MacroAssembler::lookup_interface_method()). It would be nice to keep the 
implementation unified between itable and MethodHandle linkToInterface 
linker stubs.

What MacroAssembler::lookup_interface_method(..., true 
/*return_method*/) does is interface method lookup w/o proper subtype 
check and it is equivalent to fast loop in 
MacroAssembler::lookup_interface_method_in_stub().

As a possible path forward, you could introduce the fast path check 
first by moving the fast path check into 
VtableStubs::create_itable_stub() and guard the first path over the 
itable. It would make the type checking pass over itable optional based 
on runtime check.

Then you could refactor MacroAssembler::lookup_interface_method() to 
optionally do REFC and DECC checks on every iteration and migrate 
VtableStubs::create_itable_stub()  and 
MethodHandles::generate_method_handle_dispatch() to it.

Best regards,
Vladimir Ivanov

On 14.09.2020 13:52, kuaiwei wrote:
> Now itable_stub will go through instanceKlass's itable twice to look up a method entry. resolved klass is used for type
> checking and method holder klass is used to find method entry. In many cases , we observed resolved klass is as same as
> holder klass. So we can improve itable stub based on it. If they are same klass, stub uses a fast loop to check only
> one klass. If not, a slow loop is used to checking both klasses.
> 
> Even entering in slow loop, new implementation can be better than old one in some cases. Because new stub just need go
> through itable once and reduce memory operations.
> 
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8253049
> 
> -------------
> 
> Commit messages:
>   - 8253049: Enhance itable_stub for AArch64 and x86_64
> 
> Changes: https://git.openjdk.java.net/jdk/pull/128/files
>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=128&range=00
>    Issue: https://bugs.openjdk.java.net/browse/JDK-8253049
>    Stats: 220 lines in 7 files changed: 172 ins; 35 del; 13 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/128.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/128/head:pull/128
> 
> PR: https://git.openjdk.java.net/jdk/pull/128
>