8202449: overflow handling in Random.doubles

rgiulietti · jddarcy · commit c8cff1bd6f98 · 2022-06-08T16:28:18.000Z
Reviewed-by: darcy
diff --git a/src/java.base/share/classes/java/util/Random.java b/src/java.base/share/classes/java/util/Random.java
@@ -256,7 +256,8 @@ public String toString() {
     private static final long addend = 0xBL;
     private static final long mask = (1L << 48) - 1;
 
-    private static final double DOUBLE_UNIT = 0x1.0p-53; // 1.0 / (1L << 53)
+    private static final double DOUBLE_UNIT = 0x1.0p-53; // 1.0 / (1L << Double.PRECISION)
+    private static final float FLOAT_UNIT = 0x1.0p-24f; // 1.0f / (1 << Float.PRECISION)
 
     /**
      * Creates a new random number generator. This constructor sets
@@ -598,12 +599,12 @@ public boolean nextBoolean() {
      * low-order bit of the significand would be 0 than that it would be 1.]
      *
      * @return the next pseudorandom, uniformly distributed {@code float}
-     *         value between {@code 0.0} and {@code 1.0} from this
+     *         value between {@code 0.0f} and {@code 1.0f} from this
      *         random number generator's sequence
      */
     @Override
     public float nextFloat() {
-        return next(24) / ((float)(1 << 24));
+        return next(Float.PRECISION) * FLOAT_UNIT;
     }
 
     /**
@@ -644,7 +645,7 @@ public float nextFloat() {
      */
     @Override
     public double nextDouble() {
-        return (((long)(next(26)) << 27) + next(27)) * DOUBLE_UNIT;
+        return (((long)(next(Double.PRECISION - 27)) << 27) + next(27)) * DOUBLE_UNIT;
     }
 
     private double nextNextGaussian;
@@ -1070,26 +1071,12 @@ public DoubleStream doubles() {
      * pseudorandom {@code double} values, each conforming to the given origin
      * (inclusive) and bound (exclusive).
      *
-     * <p>A pseudorandom {@code double} value is generated as if it's the result
-     * of calling the following method with the origin and bound:
-     * <pre> {@code
-     * double nextDouble(double origin, double bound) {
-     *   double r = nextDouble();
-     *   r = r * (bound - origin) + origin;
-     *   if (r >= bound) // correct for rounding
-     *     r = Math.nextDown(bound);
-     *   return r;
-     * }}</pre>
-     *
      * @param streamSize the number of values to generate
      * @param randomNumberOrigin the origin (inclusive) of each random value
      * @param randomNumberBound the bound (exclusive) of each random value
      * @return a stream of pseudorandom {@code double} values,
      *         each with the given origin (inclusive) and bound (exclusive)
-     * @throws IllegalArgumentException if {@code streamSize} is less than zero,
-     *         or {@code randomNumberOrigin} is not finite,
-     *         or {@code randomNumberBound} is not finite, or {@code randomNumberOrigin}
-     *         is greater than or equal to {@code randomNumberBound}
+    * @throws IllegalArgumentException {@inheritDoc}
      * @since 1.8
      */
     @Override
@@ -1101,27 +1088,15 @@ public DoubleStream doubles(long streamSize, double randomNumberOrigin, double r
      * Returns an effectively unlimited stream of pseudorandom {@code
      * double} values, each conforming to the given origin (inclusive) and bound
      * (exclusive).
-     *
-     * <p>A pseudorandom {@code double} value is generated as if it's the result
-     * of calling the following method with the origin and bound:
-     * <pre> {@code
-     * double nextDouble(double origin, double bound) {
-     *   double r = nextDouble();
-     *   r = r * (bound - origin) + origin;
-     *   if (r >= bound) // correct for rounding
-     *     r = Math.nextDown(bound);
-     *   return r;
-     * }}</pre>
-     *
+
      * @implNote This method is implemented to be equivalent to {@code
      * doubles(Long.MAX_VALUE, randomNumberOrigin, randomNumberBound)}.
      *
      * @param randomNumberOrigin the origin (inclusive) of each random value
      * @param randomNumberBound the bound (exclusive) of each random value
      * @return a stream of pseudorandom {@code double} values,
      *         each with the given origin (inclusive) and bound (exclusive)
-     * @throws IllegalArgumentException if {@code randomNumberOrigin}
-     *         is greater than or equal to {@code randomNumberBound}
+     * @throws IllegalArgumentException {@inheritDoc}
      * @since 1.8
      */
     @Override
diff --git a/src/java.base/share/classes/java/util/SplittableRandom.java b/src/java.base/share/classes/java/util/SplittableRandom.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,7 +24,6 @@
  */
 package java.util;
 
-import java.math.BigInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.random.RandomGenerator;
 import java.util.random.RandomGenerator.SplittableGenerator;
@@ -551,9 +550,7 @@ public DoubleStream doubles() {
      * @param randomNumberBound the bound (exclusive) of each random value
      * @return a stream of pseudorandom {@code double} values,
      *         each with the given origin (inclusive) and bound (exclusive)
-     * @throws IllegalArgumentException if {@code streamSize} is
-     *         less than zero, or {@code randomNumberOrigin}
-     *         is greater than or equal to {@code randomNumberBound}
+     * @throws IllegalArgumentException {@inheritDoc}
      */
     @Override
     public DoubleStream doubles(long streamSize, double randomNumberOrigin, double randomNumberBound) {
@@ -572,8 +569,7 @@ public DoubleStream doubles(long streamSize, double randomNumberOrigin, double r
      * @param randomNumberBound the bound (exclusive) of each random value
      * @return a stream of pseudorandom {@code double} values,
      *         each with the given origin (inclusive) and bound (exclusive)
-     * @throws IllegalArgumentException if {@code randomNumberOrigin}
-     *         is greater than or equal to {@code randomNumberBound}
+     * @throws IllegalArgumentException {@inheritDoc}
      */
     @Override
     public DoubleStream doubles(double randomNumberOrigin, double randomNumberBound) {
diff --git a/src/java.base/share/classes/java/util/random/RandomGenerator.java b/src/java.base/share/classes/java/util/random/RandomGenerator.java
@@ -540,10 +540,7 @@ default float nextFloat(float bound) {
      *
      * @throws IllegalArgumentException if {@code origin} is not finite,
      *         or {@code bound} is not finite, or {@code origin}
-     *         is greater than or equal to {@code bound}, or
-     *         the difference between {@code bound} and {@code origin}
-     *         is so large that it cannot be represented as a finite
-     *         {@code float} value
+     *         is greater than or equal to {@code bound}
      *
      * @implSpec The default implementation verifies that the {@code origin}
      *           and {@code bound} are valid then invokes {@code nextFloat()}
@@ -606,11 +603,8 @@ default double nextDouble(double bound) {
      *
      * @throws IllegalArgumentException if {@code origin} is not finite,
      *         or {@code bound} is not finite, or {@code origin}
-     *         is greater than or equal to {@code bound}, or
-     *         the difference between {@code bound} and {@code origin}
-     *         is so large that it cannot be represented as a finite
-     *         {@code double} value
-     *
+     *         is greater than or equal to {@code bound}
+
      * @implSpec The default implementation verifies that the {@code origin}
      *           and {@code bound} are valid, then invokes {@code nextDouble()}
      *           scaling and translating the result to fit between {@code origin}
diff --git a/src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java b/src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java
@@ -145,7 +145,7 @@ public static void checkStreamSize(long streamSize) {
      * @throws IllegalArgumentException if {@code bound} fails to be positive and finite
      */
     public static void checkBound(float bound) {
-        if (!(bound > 0.0 && bound < Float.POSITIVE_INFINITY)) {
+        if (!(0.0f < bound && bound < Float.POSITIVE_INFINITY)) {
             throw new IllegalArgumentException(BAD_FLOATING_BOUND);
         }
     }
@@ -158,7 +158,7 @@ public static void checkBound(float bound) {
      * @throws IllegalArgumentException if {@code bound} fails to be positive and finite
      */
     public static void checkBound(double bound) {
-        if (!(bound > 0.0 && bound < Double.POSITIVE_INFINITY)) {
+        if (!(0.0 < bound && bound < Double.POSITIVE_INFINITY)) {
             throw new IllegalArgumentException(BAD_FLOATING_BOUND);
         }
     }
@@ -195,11 +195,13 @@ public static void checkBound(long bound) {
      * @param origin the least value (inclusive) in the range
      * @param bound  the upper bound (exclusive) of the range
      *
-     * @throws IllegalArgumentException if {@code origin} is not finite, {@code bound} is not finite,
-     *                                  or {@code bound - origin} is not finite
+     * @throws IllegalArgumentException if {@code origin} is not finite,
+     *          or {@code bound} is not finite, or {@code origin}
+     *          is greater than or equal to {@code bound}
      */
     public static void checkRange(float origin, float bound) {
-        if (!(origin < bound && (bound - origin) < Float.POSITIVE_INFINITY)) {
+        if (!(Float.NEGATIVE_INFINITY < origin && origin < bound &&
+                bound < Float.POSITIVE_INFINITY)) {
             throw new IllegalArgumentException(BAD_RANGE);
         }
     }
@@ -210,11 +212,13 @@ public static void checkRange(float origin, float bound) {
      * @param origin the least value (inclusive) in the range
      * @param bound  the upper bound (exclusive) of the range
      *
-     * @throws IllegalArgumentException if {@code origin} is not finite, {@code bound} is not finite,
-     *                                  or {@code bound - origin} is not finite
+     * @throws IllegalArgumentException if {@code origin} is not finite,
+     *          or {@code bound} is not finite, or {@code origin}
+     *          is greater than or equal to {@code bound}
      */
     public static void checkRange(double origin, double bound) {
-        if (!(origin < bound && (bound - origin) < Double.POSITIVE_INFINITY)) {
+        if (!(Double.NEGATIVE_INFINITY < origin && origin < bound &&
+                bound < Double.POSITIVE_INFINITY)) {
             throw new IllegalArgumentException(BAD_RANGE);
         }
     }
@@ -343,8 +347,8 @@ public static int[] convertSeedBytesToInts(byte[] seed, int n, int z) {
      * This is the form of {@link RandomGenerator#nextLong() nextLong}() used by
      * a {@link LongStream} {@link Spliterator} and by the public method
      * {@link RandomGenerator#nextLong(long, long) nextLong}(origin, bound). If
-     * {@code origin} is greater than {@code bound}, then this method simply
-     * calls the unbounded version of
+     * {@code origin} is greater than or equal to {@code bound},
+     * then this method simply calls the unbounded version of
      * {@link RandomGenerator#nextLong() nextLong}(), choosing pseudorandomly
      * from among all 2<sup>64</sup> possible {@code long} values}, and
      * otherwise uses one or more calls to
@@ -508,8 +512,8 @@ public static long boundedNextLong(RandomGenerator rng, long bound) {
      * This is the form of {@link RandomGenerator#nextInt() nextInt}() used by
      * an {@link IntStream} {@link Spliterator} and by the public method
      * {@link RandomGenerator#nextInt(int, int) nextInt}(origin, bound). If
-     * {@code origin} is greater than {@code bound}, then this method simply
-     * calls the unbounded version of
+     * {@code origin} is greater than or equal to {@code bound},
+     * then this method simply calls the unbounded version of
      * {@link RandomGenerator#nextInt() nextInt}(), choosing pseudorandomly
      * from among all 2<sup>64</sup> possible {@code int} values}, and otherwise
      * uses one or more calls to {@link RandomGenerator#nextInt() nextInt}() to
@@ -604,8 +608,8 @@ public static int boundedNextInt(RandomGenerator rng, int bound) {
      * used by a {@link DoubleStream} {@link Spliterator} and by the public
      * method
      * {@link RandomGenerator#nextDouble(double, double) nextDouble}(origin, bound).
-     * If {@code origin} is greater than {@code bound}, then this method simply
-     * calls the unbounded version of
+     * {@code origin} is greater than or equal to {@code bound},
+     * then this method simply calls the unbounded version of
      * {@link RandomGenerator#nextDouble() nextDouble}(), and otherwise scales
      * and translates the result of a call to
      * {@link RandomGenerator#nextDouble() nextDouble}() so that it lies between
@@ -643,9 +647,15 @@ public static int boundedNextInt(RandomGenerator rng, int bound) {
     public static double boundedNextDouble(RandomGenerator rng, double origin, double bound) {
         double r = rng.nextDouble();
         if (origin < bound) {
-            r = r * (bound - origin) + origin;
+            if (bound - origin < Double.POSITIVE_INFINITY) {
+                r = r * (bound - origin) + origin;
+            } else {
+                /* avoids overflow at the cost of 3 more multiplications */
+                double halfOrigin = 0.5 * origin;
+                r = (r * (0.5 * bound - halfOrigin) + halfOrigin) * 2.0;
+            }
             if (r >= bound)  // may need to correct a rounding problem
-                r = Math.nextAfter(bound, origin);
+                r = Math.nextDown(bound);
         }
         return r;
     }
@@ -686,8 +696,8 @@ public static double boundedNextDouble(RandomGenerator rng, double bound) {
      * by a {@link Stream<Float>} {@link Spliterator} (if there were any) and by
      * the public method
      * {@link RandomGenerator#nextFloat(float, float) nextFloat}(origin, bound).
-     * If {@code origin} is greater than {@code bound}, then this method simply
-     * calls the unbounded version of
+     * {@code origin} is greater than or equal to {@code bound},
+     * then this method simply calls the unbounded version of
      * {@link RandomGenerator#nextFloat() nextFloat}(), and otherwise scales and
      * translates the result of a call to
      * {@link RandomGenerator#nextFloat() nextFloat}() so that it lies between
@@ -715,9 +725,15 @@ public static double boundedNextDouble(RandomGenerator rng, double bound) {
     public static float boundedNextFloat(RandomGenerator rng, float origin, float bound) {
         float r = rng.nextFloat();
         if (origin < bound) {
-            r = r * (bound - origin) + origin;
+            if (bound - origin < Float.POSITIVE_INFINITY) {
+                r = r * (bound - origin) + origin;
+            } else {
+                /* avoids overflow at the cost of 3 more multiplications */
+                float halfOrigin = 0.5f * origin;
+                r = (r * (0.5f * bound - halfOrigin) + halfOrigin) * 2.0f;
+            }
             if (r >= bound) // may need to correct a rounding problem
-                r = Float.intBitsToFloat(Float.floatToIntBits(bound) - 1);
+                r = Math.nextDown(r);
         }
         return r;
     }
@@ -749,7 +765,7 @@ public static float boundedNextFloat(RandomGenerator rng, float bound) {
         float r = rng.nextFloat();
         r = r * bound;
         if (r >= bound) // may need to correct a rounding problem
-            r = Float.intBitsToFloat(Float.floatToIntBits(bound) - 1);
+            r = Math.nextDown(r);
         return r;
     }
 
diff --git a/test/jdk/java/util/Random/RandomNextDoubleBoundary.java b/test/jdk/java/util/Random/RandomNextDoubleBoundary.java
@@ -24,16 +24,24 @@
 /*
  * @test
  * @summary Verify nextDouble stays within range
- * @bug 8280550 8280950 8281183
+ * @bug 8280550 8280950 8281183 8202449
+ *
+ * @key randomness
+ * @library /test/lib
+ * @build jdk.test.lib.RandomFactory
+ * @run main RandomNextDoubleBoundary
+
  */
 
 import java.util.SplittableRandom;
 import java.util.random.RandomGenerator;
+import jdk.test.lib.RandomFactory;
 
 public class RandomNextDoubleBoundary {
     public static void main(String... args) {
         negativeBounds();
         positiveBounds();
+        nextDoubleHugeRange();
     }
 
     private static void negativeBounds() {
@@ -86,9 +94,31 @@ public long nextLong() {
         }
     }
 
+    public static void nextDoubleHugeRange() {
+        var random = RandomFactory.getRandom();
+        var n = 100_000;
+
+        var origin = -(3.0 / 4.0) * Double.MAX_VALUE;
+        var bound = (3.0 / 4.0) * Double.MAX_VALUE;
+        assertTrue(bound - origin == Double.POSITIVE_INFINITY);
+
+        /* all are within [origin, bound) */
+        assertTrue(random.doubles(n, origin, bound)
+                .allMatch(d -> origin <= d && d < bound));
+
+        /* some are near the origin */
+        assertTrue(random.doubles(n, origin, bound)
+                .anyMatch(d -> d < (15.0 / 16.0) * origin));
+
+        /* some are near the bound */
+        assertTrue(random.doubles(n, origin, bound)
+                .anyMatch(d -> d > (15.0 / 16.0) * bound));
+    }
+
     public static void assertTrue(boolean condition) {
         if (!condition) {
             throw new AssertionError();
         }
     }
+
 }
