RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable

[Phil Race]

On Thu, 13 Jan 2022 08:25:22 GMT, Andrey Turbanov <aturbanov at openjdk.org> wrote:

> Method `Class.isAssignableFrom` is often used in form of:
> 
>     if (clazz.isAssignableFrom(obj.getClass())) {
> Such condition could be simplified to more shorter and performarnt code
> 
>     if (clazz.isInstance(obj)) {
>     
> Replacement is equivalent if it's known that `obj != null`.
> In JDK codebase there are many such places.
> 
> I tried to measure performance via JMH
> 
>     Class<?> integerClass = Integer.class;
>     Class<?> numberClass = Number.class;
> 
>     Object integerObject = 45666;
>     Object doubleObject = 8777d;
> 
>     @Benchmark
>     public boolean integerInteger_isInstance() {
>         return integerClass.isInstance(integerObject);
>     }
> 
>     @Benchmark
>     public boolean integerInteger_isAssignableFrom() {
>         return integerClass.isAssignableFrom(integerObject.getClass());
>     }
> 
>     @Benchmark
>     public boolean integerDouble_isInstance() {
>         return integerClass.isInstance(doubleObject);
>     }
> 
>     @Benchmark
>     public boolean integerDouble_isAssignableFrom() {
>         return integerClass.isAssignableFrom(doubleObject.getClass());
>     }
> 
>     @Benchmark
>     public boolean numberDouble_isInstance() {
>         return numberClass.isInstance(doubleObject);
>     }
> 
>     @Benchmark
>     public boolean numberDouble_isAssignableFrom() {
>         return numberClass.isAssignableFrom(doubleObject.getClass());
>     }
> 
>     @Benchmark
>     public boolean numberInteger_isInstance() {
>         return numberClass.isInstance(integerObject);
>     }
> 
>     @Benchmark
>     public boolean numberInteger_isAssignableFrom() {
>         return numberClass.isAssignableFrom(integerObject.getClass());
>     }
> 
>     @Benchmark
>     public void numberIntegerDouble_isInstance(Blackhole bh) {
>         bh.consume(numberClass.isInstance(integerObject));
>         bh.consume(numberClass.isInstance(doubleObject));
>     }
> 
>     @Benchmark
>     public void integerIntegerDouble_isAssignableFrom(Blackhole bh) {
>         bh.consume(integerClass.isAssignableFrom(integerObject.getClass()));
>         bh.consume(integerClass.isAssignableFrom(doubleObject.getClass()));
>     }
> 
> `isInstance` is a bit faster than `isAssignableFrom`
> 
> Benchmark                              Mode  Cnt  Score   Error  Units
> integerDouble_isAssignableFrom         avgt    5  1,173 ± 0,026  ns/op
> integerDouble_isInstance               avgt    5  0,939 ± 0,038  ns/op
> integerIntegerDouble_isAssignableFrom  avgt    5  2,106 ± 0,068  ns/op
> numberIntegerDouble_isInstance         avgt    5  1,516 ± 0,046  ns/op
> integerInteger_isAssignableFrom        avgt    5  1,175 ± 0,029  ns/op
> integerInteger_isInstance              avgt    5  0,886 ± 0,017  ns/op
> numberDouble_isAssignableFrom          avgt    5  1,172 ± 0,007  ns/op
> numberDouble_isInstance                avgt    5  0,891 ± 0,030  ns/op
> numberInteger_isAssignableFrom         avgt    5  1,169 ± 0,014  ns/op
> numberInteger_isInstance               avgt    5  0,887 ± 0,016  ns/op

I've stared at the javadoc for Class.isAssignableFrom and Class.isInstance and if a non-null instance is used to get a non-null class they are PROBABLY the same but it is far from clear. The implementations of both are at least native and may be instrinsicified. The doc for Class.isAssignableFrom cites JLS 5.1.4 which in what I found is about primitives so I suspect it is woefully out of date 
https://docs.oracle.com/javase/specs/jls/se17/html/jls-5.html#jls-5.1.4

What client tests have you run that touch the code you are changing ?

In short I see insufficient value in the changes here and would prefer you drop the client part so I don't have to worry about it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7061