StrictMath performance improvement not ported to Math?

[Joe Darcy]

Hello Martin,

On 11/18/2011 6:29 AM, Martin Desruisseaux wrote:
> Hello all
>
> On December 1, 2010, "darcy" committed a slight performance 
> improvement to the StrictMath.min/max methods with floating point 
> arguments (commit 8aabca72877c). The calls to the 
> doubleToLongBits(double) method were replaced by calls to the 
> doubleToRawLongBits(double) method, and similarly for the float type. 
> Since the call to doubleToLongBits was used only in order to determine 
> if an argument was negative zero, and since NaN can not map to the 
> bits pattern of -0.0, the extra cost of collapsing all NaN values to a 
> single canonical NaN (which is the only difference between 
> doubleToLongBits and doubleToRawLongBits) was unnecessary.
>
> However this improvement has not been ported from StrictMath to Math; 

Good catch and mea culpa!

> the more widely-used Math class still invokes the (presumed) slower 
> doubleToLongBits(double) method. Actually it appears that most Math / 
> StrictMath method implementations delegate to the other class: some 
> Math methods delegates to StrictMath (mostly the methods backed by 
> native code), and some StrictMath methods delegate to Math (mostly the 
> methods implemented in pure-Java). The min/max methods are an 
> exception; their implementation is copied in both classes. Maybe this 
> explain why the code was updated in only one class and not the other?

Yes; I may have assumed one of Math.{min, max} and StrictMath.{min, max} 
delegated to the other since, as you note, that is the usual pattern in 
these classes.

>
> In the patch submitted below, I propose the following changes:
>
> 1) Ported the doubleToLongBits --> doubleToRawLongBits changes from 
> StrictMath to Math.
>
> 2) Replaced StrictMath implementations by calls to Math, in order to 
> reduce the risk that the code of only one class is updated in the future.
>
> 3) In if statements, replaced:
>
>       (a == 0.0d) && (Double.doubleToLongBits(a) == 
> negativeZeroDoubleBits)
> by
>       (Double.doubleToLongBits(a) == negativeZeroDoubleBits)
>
> since the later implies the former.

The performance properties of the two versions of the code may differ 
depending on the frequency of zeros in the input and the cost of the 
bitwise conversion operation.  I'd prefer to leave the code logic as-is 
in absence of some benchmarking that showed a helpful difference.


>
> 4) Moved the check for (a != a) from the method beginning to the last 
> statement, which is tested only if (a <= b) were false rather than 
> tested unconditionally in every cases. I'm not sure if it make a real 
> performance difference however.
>
> Webrev link: http://webrev.geomatys.com/Math/min_max/index.html
>
>     Regards,
>
>

I'd prefer to see a webrev with:

* All min/max logic from StrictMath moved into math, including for the 
integral types int and long
* All StrictMath min/max methods delegating to their Math counterpart
* Verification all java/lang/Math and java/lang/StrictMath regression 
tests still pass

Thanks,

-Joe