RFR (S) 8073479: Replace obj.getClass hacks with Objects.requireNonNull

[Vitaly Davidovich]

The problem is microbenchmarking this may not tell the true story (like all
microbenchmarks).  In this case the explicit test and branch is probably
perfectly predicted because there's not much code.  In a bigger
application, where the history fell out of the buffer, it'll depend on what
static prediction is used.  Or, this branch will evict another useful one
from the history buffer.  Not saying you're wrong in your suggestion to go
ahead with this change, but something to keep in mind.

sent from my phone
On Feb 19, 2015 1:27 PM, "Aleksey Shipilev" <aleksey.shipilev at oracle.com>
wrote:

> On 02/19/2015 06:17 PM, Peter Levart wrote:
> > On 02/19/2015 04:02 PM, Aleksey Shipilev wrote:
> >>> >Hi Aleksey,
> >>> >
> >>> >Is profile pollution problem already fixed? Can Objects.requireNonNull
> >>> >be used in performance critical code without fear of performance
> >>> >regression?
> >> What profile pollution? Objects.requireNonNull is a static method. See
> >> also the link in bug description:
> >>    http://cr.openjdk.java.net/~shade/scratch/NullChecks.java
> >>
> >> -Aleksey.
> >>
> >
> > Remi described it here:
> >
> >
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-April/026669.html
>
> Okay, Remi's premise is that implicit null check is much cheaper than an
> explicit one when it's not tripped. It's then seem to extend to mean
> that pessimistically inflating the nullcheck into the explicit one costs
> performance, and therefore one should avoid requireNonNull.
>
> But then we all got confused with benchmarks and whatnot. The benchmark
> from JDK-8047127 actually measures the cost of raising the exceptions,
> where getClass and friends are profoundly cheating:
>   https://bugs.openjdk.java.net/browse/JDK-8042127#comment-13611255
>   http://cr.openjdk.java.net/~shade/8042127/NullCheckMix.java
>
> However, if you do the actual benchmark that compares explicit NP checks
> vs implicit NP checks (that benchmark is very tricky to get right):
>  http://cr.openjdk.java.net/~shade/scratch/NullCheckPollution.java
>
> Benchmark         (count)  (pollute)  Mode  Cnt  Score   Error  Units
> NCP.testExplicit   100000      false  avgt   50  1.813 ± 0.004  ns/op
> NCP.testExplicit   100000       true  avgt   50  1.891 ± 0.029  ns/op
> NCP.testImplicit   100000      false  avgt   50  1.812 ± 0.003  ns/op
> NCP.testImplicit   100000       true  avgt   50  1.811 ± 0.001  ns/op
>
> There, testExplicit with pollution is the only one that has explicit
> test+branch. The difference is minuscule (< 1/3 of CPU cycle), and I
> would choose not to care, even for JDK code.
>
> That test is crafted specifically so that compiler could not
> disambiguate different callers. In real life, a Sufficiently Smart
> Compiler may inline the callees and reprofile in the context of caller.
> But even in the absence of that, the performance hit would be buried in
> everything else.
>
> In the places where you *need* the every last bit of performance, you
> can do the by-hand explicit nullcheck. (Note that it is not a good idea
> to do getClass anyway, because of JDK-8073432).
>
> Thanks,
> -Aleksey.
>
>