Request for review (S): C2 should recognize "obj.getClass() == A.class" code pattern

[Rémi Forax]

On 04/24/2012 06:16 PM, Krystal Mok wrote:
> Hi all,
>
> https://gist.github.com/2481025
>
> Could I get some reviews for this patch, please? Any comments and 
> suggestions are welcomed.
> I'm still pretty new to C2, and I'm taking this as an exercise to get 
> more familiar with the implementation details.
>
> The idea is to optimize this code pattern:
>
> x.getClass() == A.class
>
> into
>
> x._klass == klassof(A)
>
> which shortens the original pattern by 1 load.
>
> A scenario where this might be useful is the following code pattern:
>
>   if (x.getClass() == A.class) {
>     A a = (A) x; // checkcast
>     a.m();       // ... use a ...
>   }
>
> Before this change, there would be redundant exact type check in the 
> generated code, as mentioned in an earlier thread [1].
> After this change, the redundant check gets optimized away, because 
> the new pattern is the same as the one used in the fast path of 
> instanceof/Class.isInstance()/checkcast.
>
> It's probably rare to see this pattern in normal Java code, where 
> normal virtual dispatch should be favored instead. But it might be a 
> common pattern in dynamic language runtime implementations.
> I'll leave it to Remi to explain the original motivation for 
> optimizing this code pattern :-)

This pattern is used to implement an inlining cache using invokedynamic,
it's used in most of the dynamic runtimes that used invokedynamic, 
JRuby, Groovy and
Mark Roos's SmallTalk to cite a few.
With this patch, implementing invokeinterface with invokedynamic 
produces the exact same code
as invokeinterface which demonstrate that after being JITed 
invokedynamic has no overhead.

>
> Tested with CTW -XX:+VerifyIterativeGVN, and SPECjvm2008.
> The composite result in SPECjvm2008 seems to have dropped a little 
> bit, I'm not sure if that's caused by this change or by some variance 
> on my test machine.
>
> Regards,
> Kris

cheers,
Rémi