Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-6667086

Double.doubleToLongBits(final double value) contains inefficient test for NaN

    XMLWordPrintable

    Details

    • Type: Enhancement
    • Status: Resolved
    • Priority: P5
    • Resolution: Fixed
    • Affects Version/s: 6
    • Fix Version/s: 9
    • Component/s: core-libs
    • Labels:
    • Subcomponent:
    • Resolved In Build:
      b02
    • CPU:
      x86
    • OS:
      windows_xp

      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.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              bpb Brian Burkhalter
              Reporter:
              ndcosta Nelson Dcosta (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Imported:
                Indexed: