JDK-8074306 : NULLCHK is emitted as Object.getClass
  • Type: Enhancement
  • Component: tools
  • Sub-Component: javac
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2015-03-03
  • Updated: 2015-06-04
  • Resolved: 2015-03-06
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9 b55Fixed
Related Reports
Relates :  
JDK-8073432 - Object.getClass() throws stackless NPE, due to C2 intrinsic
Relates :  
JDK-8042127 - Performance issues with java.util.Objects.requireNonNull
Description
Originally reported here: 
 https://twitter.com/rafaelcodes/status/572718220047728640

Indeed, the genNullCheck does:

     /** Generate a null check from the object value at stack top. */
    private void genNullCheck(DiagnosticPosition pos) {
        callMethod(pos, syms.objectType, names.getClass,
                   List.<Type>nil(), false);
         code.emitop0(pop);
     }

This may cause the debugging problems due to JDK-8073432.
We might want to switch to Objects.requireNonNull, or generating the explicit NPE check instead.
Comments
I believe using Objects.requireNonNull is the right solution; yes, it adds few more constants (1 method constant per classfile, which will in turn point to a class and a name entry - in total 6 entries per classfile), which I'd say is negligible (given also the fact that javac tends to generate such checks quite rarely). From the performance discussion you linked, it seems that whatever magic sauce will be added to the VM to make Objects.requireNonNull fast, it would be better for javac to leverage on that too - or we could have weird performance cliffs for javac-generated NPE checks, if they are implemented using some other mechanism. Btw, the currently implemented approach (getClass()), generates CP entries for the invokevirtual on getClass() - so there's a cost in term of CP with that strategy too; the only difference there is that, since Object is likely mentioned in most Java classes, the entry for the receiver class will likely be there already. But we are really talking about minor stuff, I believe - i.e. 1/2 additional entries per classfile where the number of classfles touched will be negligible (in the langtools repo, the ratio of classes in which a synthetic null check is generated is 21:2189).
03-03-2015
Objects.requireNonNull works for me, I'm just listing all the caveats :)
03-03-2015
There are also potential minor performance issues, see JDK-8042127.
03-03-2015
The proposed change looks ok - I have come up with something similar myself - the only difference is that I need we need to check for the 'target' option - if target < 7 then the usual NPE check should be emitted.
03-03-2015
It should be noted that switching to either Objects.rNN or explicit exceptions might increase the constant pool size. Maybe the "proper" way would be to enable javac to piggyback in implicit NPE checks for the expressions themselves?
03-03-2015
Prototype change with Objects.requireNonNull: http://cr.openjdk.java.net/~shade/8074306/webrev.OrNN.1/
03-03-2015