JDK-6667086 : Double.doubleToLongBits(final double value) contains inefficient test for NaN
  • Type: Enhancement
  • Component: core-libs
  • Sub-Component: java.lang
  • Affected Version: 6
  • Priority: P5
  • Status: Resolved
  • Resolution: Fixed
  • OS: windows_xp
  • CPU: x86
  • Submitted: 2008-02-25
  • Updated: 2014-09-26
  • Resolved: 2014-01-17
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 b02Fixed
Related Reports
Relates :  
Relates :  
Description
A DESCRIPTION OF THE REQUEST :
The current implementation of Double.doubleToLongBits(final double value) uses a too complex expression to test for an NaN, that involves the generation of a conditional branch (with bad branch prediction in CPU), and loading large constants from the constants pool at runtime.

Also Math.abs(double) is unnecessarily complicate for NaN.

JUSTIFICATION :
Of course, it could be rewritten as:

    public static long doubleToLongBits(final double value) {
        final long result;
        /*
         * Check for NaN based on values of bit fields, maximum exponent and
         * nonzero significand: if all bits in exponent field are not set,
         * the value is finite or denormal, so it is unchanged; otherwise if
         * the significand bits are all zeroes, the value is infinite, so it
         * is unchanged; otherwise it is representing an NaN, and it is
         * replaced by a fixed long.
         */
        return ((result = doubleToRawLongBits(value)) & DoubleConsts.EXP_BIT_MASK) == DoubleConsts.EXP_BIT_MASK
                && (result & DoubleConsts.SIGNIF_BIT_MASK) != 0L
                ? 0x7ff8000000000000L : result;
    }

But this still makes an unnecessary store to a temporary variable, and the JVM will possibly delay the evaluation of the load at runtime.

Why not simply rewriting it as:

    public static long doubleToLongBits(final double value) {
        if (!isNaN(value))
            return value; // Not NaN, branch not taken
       return 0x7ff8000000000000L; // canonical non-signaling NaN(+0)
   }

Or even simplier (using faster return for not NaN to help branch prediction: no conditional branch is taken for non-NaN values):

    public static long doubleToLongBits(final double value) {
        if (value == value)
            return value; // Not NaN, branch not taken
       return 0x7ff8000000000000L; // canonical non-signaling NaN(+0)
   }

---------------

Note also, that we do have:  -Double.NaN == Double.NaN
but we can still have: Double.compare(-Double.NaN, Double.NaN) == 0 despite their bit patterns are still different (tested on Windows 32bit platform for x86, both Intel and AMD).

In other words, the negation unary operator does not necessarily return a canonical NaN (except in strictfp mode where all NaNs are canonicalized).

This does not seem to affect the semantic of operations, except in non-strictfp mode, where one would incorrectly expect that:

   Compare(a, b) == 0

implies:

   Double.doubleToRawLongBits(a) == Double.doubleToRawLongBits(b)

Example (using signaled signaling NaN(+1) and NaN(-1) with most significant bit of significand set, but I could use the long constant for non-signaled signaling NaNs, with this bit cleared but getting immediately set during copies). Demonstration in source code below.

This also occurs with Nan(-0). Not a bug for me, but may be intrigating. The (non-strictfp) negation of a NaN effectively switches the sign bit 63, and preserves the value of the signal in bits 50..0, but sets NaN to the already signaled state by forcing bit 51 when the signal is not null. When the signal is null, it still has a sign bit which is considered.

In strictfp mode, the value set is expected to be restricted to contain ONLY a single canonicalized NaN, but this is not enforced: just the sign bit 63 is cleared, the signaled state bit 51 is set, and the signal value (the rest of the significand in bits 50..0) is kept.


EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
0
fff8000000000001
0
fff8000000000001 // sign bit of NaNs preserved by non-strictfp "-"
0
7ff8000000000000 // NaNs canonicalized by strictfp "-"

ACTUAL -
0
fff8000000000001
0
fff8000000000001 // sign bit of NaNs preserved by non-strictfp "-"
0
7ff8000000000001 // sign bit of NaNs cleared by strictfp "-"


---------- BEGIN SOURCE ----------
public static class Test {
    static strictfp double negate(double x) {
        return -x; // due to strictfp, bit sign is inversed except if isNaN(x) .
    }
    // Should be equivalent (but slower) code:
    // static double negate(double x) {
    //     if (x == x)
    //         return -x; // not NaN, may be infinite, or denormal
    //     return Double.NaN;
    // }

    public static void main(final String[] args) {
        double negativeNaN = Double.longBitsToDouble(0xfff8000000000001L);
        System.out.println(Double.compare(negativeNaN, Double.NaN));
        System.out.println(Long.toHexString(Double
                .doubleToRawLongBits(negativeNaN)));
        System.out.println(Double.compare(-(-negativeNaN), Double.NaN));
        System.out.println(Long.toHexString(Double
                .doubleToRawLongBits(-(-negativeNaN))));
        System.out.println(Double.compare(negate(negate(negativeNaN)), Double.NaN));
        System.out.println(Long.toHexString(Double
                .doubleToRawLongBits(minus(negativeNaN))));
    }

}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
For strictfp computing mode, we should have a negate(double) method in java.lang.MathStrict:

public class Math {
    public static double negate(double a) {
        return -a;
    }
}
public class MathStrict {
    public static double negate(double a) {
        if (a == a) return -a;
        return NaN;
    }
}

Or if this is really a bug, the "-" operator should be corrected so that it canonicalizes the NaNs, and so the implementation in MathStrict becomes simply:

public class MathStrict {
    public static strictfp double negate(double a) {
        return -a;
    }
}

The alternative is to use the binary "-" operator with a zero operand, as in the Math.abs(double) method.

Comments
Review thread: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-January/024455.html
16-01-2014

PUBLIC COMMENTS A related comment from core-libs-dev: http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-September/007708.html Hi. There are some possible optimizations for some methods. For nextAfter(double,double) (same for float version), instead of testing NaN-ity right away, we can test most common (or at least regular) cases first: public static double nextAfter(double start, double direction) { // Balancing out by branching to going-down case first, // for it is heavier than going-up case (test if start is +-0.0). if (start > direction) { // Going down. if (start == 0.0d) { // start is +0.0 or -0.0 return -Double.MIN_VALUE; } final long transducer = Double.doubleToRawLongBits(start); assert transducer != 0L; return Double.longBitsToDouble(transducer + ((transducer > 0L) ? -1L:1L)); } else if (start < direction) { // Going up. // Add +0.0 to get rid of a -0.0 (+0.0 + -0.0 => +0.0) // then bitwise convert start to integer. final long transducer = Double.doubleToRawLongBits(start + 0.0d); return Double.longBitsToDouble(transducer + ((transducer >= 0L) ? 1L:-1L)); } else if (start == direction) { return direction; } else { // start and/or direction is NaN return start + direction; } } Same for nextUp(double) and float version (also, testing transducer >= 0L instead of d >= 0.0D seems to help): public static double nextUp(double d) { if (d < Double.POSITIVE_INFINITY) { final long transducer = Double.doubleToRawLongBits(d + 0.0D); return Double.longBitsToDouble(transducer + ((transducer >= 0L) ? 1L:-1L)); } else { // d is NaN or +Infinity return d; } }
19-09-2011

EVALUATION Will consider.
16-09-2011