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

[Andrey Turbanov]

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

First of all, I did check that replace is definitely valid.

public static void main(String[] args) {
        List<Class<?>> classes = List.of(Integer.class, Number.class, Serializable.class,
                boolean.class, byte.class, short.class, int.class, long.class, char.class, float.class, double.class,
                Boolean.TYPE, Byte.TYPE, Short.TYPE, Integer.TYPE, Long.TYPE, Character.TYPE, Float.TYPE, Double.TYPE,
                Boolean.class, Byte.class, Short.class, Integer.class, Long.class, Character.class, Float.class, Double.class);
        List<Object> objects = List.of(
                new Integer(4), new BigInteger("345345"), new StringBuilder("df"),
                true, false, (byte)1, (byte)0, (short)1, (short)0, (int)1, (int)0, 1L, 0L, (char)1, (char)0, 1f, 0f, 1d, 0d,
                new Boolean(true), new Byte((byte)3), new Short((short)4), new Integer(5), new Long(6), new Character((char)7), new Float(8f), new Double(9d),
                new ClassInstanceAssignableCheck());
        for (Class<?> clazz : classes) {
            System.out.println("Check " + clazz);
            for (Object object : objects) {
                checkIsSave(object, clazz);
            }
            System.out.println();
        }
    }

    private static void checkIsSave(@Nonnull Object object, @Nonnull Class<?> clazz) {
        boolean isInstance = clazz.isInstance(object);
        boolean assignableFrom = clazz.isAssignableFrom(object.getClass());
        System.out.println("  " + object + " of class " + object.getClass().getSimpleName() + " check with " + clazz.getSimpleName() + ": isInstance=" + isInstance + " isAssignableFrom=" + assignableFrom);
        if (isInstance != assignableFrom) {
            throw new RuntimeException("Not matched for " + object + " and " + clazz);
        }
    }


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

I did run tier4. It includes all client tests as I know. Some of them failed, when I run in batch. But I rechecked all failed tests one-by-one and they are fine.
Can you suggest which test better to run?

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

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