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