Remove redundant calls of toString()

[Claes Redestad]

On 04/28/2014 08:57 AM, David Holmes wrote:
> On 28/04/2014 1:05 PM, Otávio Gonçalves de Santana wrote:
>> In my opinion not, because Objects.requireNonNull is more readable than
>> just string.toString. This way is more understandable which field is
>> required and doesn't impact on performance.
>
> An invocation of requireNonNull is potentially more expensive than the 
> implicit null check that happens with foo.toString().
>
> David
> ----- 

My thought was that these two would be inlined to the exact same thing, 
so I did a quick test to see what happens when you do foo.toString() 
versus Objects.requireNonNull(foo) on a set of randomly generated 
String[]'s with different amounts of null elements(0p: no null entries, 
1p: 1% null entries etc):

Benchmark                                     Mode Samples         
Mean   Mean error    Units
s.m.ThrowAwayBenchmark.nullToString0p        thrpt 6   356653.044     
3573.707   ops/ms
s.m.ThrowAwayBenchmark.nullToString1p thrpt         6   353128.903     
2764.102   ops/ms
s.m.ThrowAwayBenchmark.nullToString10p thrpt         6   297956.571     
9580.251   ops/ms
s.m.ThrowAwayBenchmark.nullToString50p thrpt         6   158172.036     
1893.096   ops/ms
s.m.ThrowAwayBenchmark.nullToString100p thrpt         6    
18194.614      472.091   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull0p thrpt         6   357855.126     
2979.090   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull1p thrpt         6    67601.134     
7004.689   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull10p thrpt         6     
8150.595      538.970   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull50p thrpt         6     
1604.919      220.903   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull100p thrpt         6      
820.626       60.752   ops/ms

Yikes! As long as the value is never null they're inlined nicely and 
neither have the upper hand performance-wise, but as soon as you get 
some null values, Objects.requireNonNull degenerates much faster than 
its foo.toString counterpart. I think this is a JIT issue - optimizing 
exception-paths might not be the highest priority, but 
Objects.requireNonNull is used pretty extensively in the JDK and my 
expectation would be that it shouldn't degrade performance when things 
actually are null now and then.

/Claes