12

I'm trying to get as much performance as possible from some internal method.

The Java code is:

List<DirectoryTaxonomyWriter> writers = Lists.newArrayList();
private final int taxos = 4;

[...]

@Override
public int getParent(final int globalOrdinal) throws IOException {
    final int bin = globalOrdinal % this.taxos;
    final int ordinalInBin = globalOrdinal / this.taxos;
    return this.writers.get(bin).getParent(ordinalInBin) * this.taxos + bin; //global parent
}

In my profiler I saw there is 1% CPU spend in java.util.Objects.requireNonNull, but I don't even call that. When inspecting the bytecode, I saw this:

 public getParent(I)I throws java/io/IOException 
   L0
    LINENUMBER 70 L0
    ILOAD 1
    ALOAD 0
    INVOKESTATIC java/util/Objects.requireNonNull (Ljava/lang/Object;)Ljava/lang/Object;
    POP
    BIPUSH 8
    IREM
    ISTORE 2

So the compiler generates this (useless?) check. I work on primitives, which cannot be null anyways, so why does the compiler generate this line? Is it a bug? Or 'normal' behaviour?

(I might work around with a bitmask, but I'm just curious)

[UPDATE]

  1. The operator seems to be having nothing to do with it (see answer below)

  2. Using the eclipse compiler (version 4.10) I get this more reasonable result:

    public getParent(I)I throws java/io/IOException 
       L0
        LINENUMBER 77 L0
        ILOAD 1
        ICONST_4
        IREM
        ISTORE 2
       L1
        LINENUMBER 78 L

So that is more logical.

Rob Audenaerde
  • 19,195
  • 10
  • 76
  • 121

3 Answers3

3

Why not?

Assuming

class C {
    private final int taxos = 4;

    public int test() {
        final int a = 7;
        final int b = this.taxos;
        return a % b;
    }
}

a call like c.test() where c is declared to as C must throw when c is null. Your method is equivalent to

    public int test() {
        return 3; // `7 % 4`
    }

as you work with constants only. With test being non-static, the check must be done. Normally, it would get done implicitly when a field gets accessed or a non-static method gets called, but you don't do it. So an explicit check is needed. One possibility is to call Objects.requireNonNull.

The bytecode

Forget not that the bytecode is basically irrelevant for the performance. The task of javac is to produce some bytecode whose execution corresponds with your source code. It's not meant to do any optimizations, as optimized code is usually longer and harder to analyze, while the bytecode is actually the source code for the optimizing JIT compiler. So javac is expected to keep it simple....

The performance

In my profiler I saw there is 1% CPU spend in java.util.Objects.requireNonNull

I'd blame the profiler first. Profiling Java is pretty hard and you can never expect perfect results.

You probably should try making the method static. You surely should read this article about null checks.

maaartinus
  • 44,714
  • 32
  • 161
  • 320
  • 1
    Thanks @maaartinus for your insightful answer. I'll surely read up your linked article. – Rob Audenaerde Feb 04 '20 at 08:25
  • 1
    “With test being non-static, the check must be done” Actually, there is no reason to test whether `this` is non-`null`. As you said yourself, a call like `c.test()` must fail when `c` is `null` and it has to fail immediately, instead of entering the method. So within `test()`, `this` can never be `null` (otherwise there would be a JVM bug). So no need to check. The actual fix should be changing the field `taxos` to `static`, as there is no point in reserving memory in every instance for a compile-time constant. Then, whether `test()` is `static` is irrelevant. – Holger Apr 17 '20 at 23:54
2

Well it seems my question was 'wrong' as it has nothing to do with the operator, but rather the field itself. Still don't know why..

   public int test() {
        final int a = 7;
        final int b = this.taxos;
        return a % b;
    }

Which turns into:

  public test()I
   L0
    LINENUMBER 51 L0
    BIPUSH 7
    ISTORE 1
   L1
    LINENUMBER 52 L1
    ALOAD 0
    INVOKESTATIC java/util/Objects.requireNonNull (Ljava/lang/Object;)Ljava/lang/Object;
    POP
    ICONST_4
    ISTORE 2
   L2
    LINENUMBER 53 L2
    BIPUSH 7
    ILOAD 2
    IREM
    IRETURN
Rob Audenaerde
  • 19,195
  • 10
  • 76
  • 121
  • 1
    Could the compiler actually be afraid that `this` references `null`? Would this be possible? – atalantus Feb 03 '20 at 15:17
  • 1
    No that does not make any sense, unless the compiler compiles the field to an `Integer` somehow, and this is the result of autoboxing? – Rob Audenaerde Feb 03 '20 at 15:18
  • 1
    Doesn't `ALOAD 0` reference `this`? So it would make sense (not really) that the compiler adds a nullcheck – Lino Feb 03 '20 at 15:19
  • Mmm, that actually makes sense in a weird way. `On instance method invocation, local variable 0 is always used to pass a reference to the object on which the instance method is being invoked (this in the Java programming language). Any parameters are subsequently passed in consecutive local variables starting from local variable 1.` – Rob Audenaerde Feb 03 '20 at 15:23
  • 1
    So the compiler is actually adding a null-check for `this`? Great :/ – Rob Audenaerde Feb 03 '20 at 15:24
  • 1
    I'll try to make a minimal piece of code with the command-line `javac` to verify tomorrow; and if that also show this behaviour, I think it might be a javac-bug? – Rob Audenaerde Feb 03 '20 at 15:30
2

Firstly, here's a minimal reproducible example of this behavior:

/**
 * OS:              Windows 10 64x
 * javac version:   13.0.1
 */
public class Test {
    private final int bar = 5;

    /**
     * public int foo();
     *   Code:
     *     0: iconst_5
     *     1: ireturn
     */
    public int foo() {
        return bar;
    }

    /**
     * public int foo2();
     *   Code:
     *     0: aload_0
     *     1: invokestatic  #13     // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
     *     4: pop
     *     5: iconst_5
     *     6: ireturn
     */
    public int foo2() {
        return this.bar;
    }
}

The behavior is because of how the Java compiler optimizes compile-time constants.

Note that in the byte code of foo() no object reference is accessed to get the value of bar. That's because it is a compile-time constant and thus the JVM can simply execute the iconst_5 operation to return this value.

When changing bar into a non-compile time constant (either by removing the final keyword or not initializing within declaration but inside the constructor) you would get:

/**
 * OS:              Windows 10 64x
 * javac version:   13.0.1
 */
public class Test2 {
    private int bar = 5;

    /**
     * public int foo();
     *   Code:
     *     0: aload_0
     *     1: getfield      #7
     *     4: ireturn
     */
    public int foo() {
        return bar;
    }

    /**
     * public int foo2();
     *   Code:
     *     0: aload_0
     *     1: getfield      #7
     *     4: ireturn
     */
    public int foo2() {
        return this.bar;
    }
}

where aload_0 pushes the reference of this onto the operand stack to then get the bar field of this object.

Here the compiler is clever enough to notice that aload_0 (the this reference in case of member functions) can logically not be null.

Now is your case actually a missing compiler optimization?

See @maaartinus answer.

atalantus
  • 721
  • 2
  • 9
  • 26