changeset 46587:6c97f34cb194

8182554: Code for os::random() assumes long is 32 bits Summary: And make updating the _rand_seed thread safe. Reviewed-by: stuefe, kbarrett, stefank
author coleenp
date Wed, 28 Jun 2017 16:14:20 -0400
parents 33bfb73333f9
children 27a438928e38
files hotspot/src/os/windows/vm/os_windows.cpp hotspot/src/share/vm/classfile/altHashing.cpp hotspot/src/share/vm/memory/metaspace.cpp hotspot/src/share/vm/prims/whitebox.cpp hotspot/src/share/vm/runtime/os.cpp hotspot/src/share/vm/runtime/os.hpp hotspot/src/share/vm/runtime/synchronizer.cpp hotspot/test/native/runtime/test_os.cpp
diffstat 8 files changed, 71 insertions(+), 58 deletions(-) [+]
line wrap: on
line diff
--- a/hotspot/src/os/windows/vm/os_windows.cpp	Tue Jun 27 12:27:27 2017 +0000
+++ b/hotspot/src/os/windows/vm/os_windows.cpp	Wed Jun 28 16:14:20 2017 -0400
@@ -2839,7 +2839,7 @@
 
 #ifdef ASSERT
   // Variable for the failure injection
-  long ran_num = os::random();
+  int ran_num = os::random();
   size_t fail_after = ran_num % bytes;
 #endif
 
--- a/hotspot/src/share/vm/classfile/altHashing.cpp	Tue Jun 27 12:27:27 2017 +0000
+++ b/hotspot/src/share/vm/classfile/altHashing.cpp	Wed Jun 28 16:14:20 2017 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2017, 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
@@ -33,7 +33,7 @@
 // return a random number, which is one of the possible hash code used for
 // objects.  We don't want to call the synchronizer hash code to install
 // this value because it may safepoint.
-intptr_t object_hash(Klass* k) {
+static intptr_t object_hash(Klass* k) {
   intptr_t hc = k->java_mirror()->mark()->hash();
   return hc != markOopDesc::no_hash ? hc : os::random();
 }
@@ -45,7 +45,7 @@
   int SEED_MATERIAL[8] = {
             (int) object_hash(SystemDictionary::String_klass()),
             (int) object_hash(SystemDictionary::System_klass()),
-            (int) os::random(),  // current thread isn't a java thread
+            os::random(),  // current thread isn't a java thread
             (int) (((julong)nanos) >> 32),
             (int) nanos,
             (int) (((julong)now) >> 32),
--- a/hotspot/src/share/vm/memory/metaspace.cpp	Tue Jun 27 12:27:27 2017 +0000
+++ b/hotspot/src/share/vm/memory/metaspace.cpp	Wed Jun 28 16:14:20 2017 -0400
@@ -4322,7 +4322,7 @@
     const int num_max_ops = num_chunks * 100;
     int num_ops = num_max_ops;
     const int average_phase_length = (int)(phase_length_factor * num_chunks);
-    int num_ops_until_switch = MAX2(1, (int)(average_phase_length + os::random() % 8 - 4));
+    int num_ops_until_switch = MAX2(1, (average_phase_length + os::random() % 8 - 4));
     bool return_phase = true;
     while (num_ops > 0) {
       int chunks_moved = 0;
@@ -4333,7 +4333,7 @@
             chunks_moved = 1;
           }
         } else {
-          const int list_length = MAX2(1, (int)(os::random() % num_ops_until_switch));
+          const int list_length = MAX2(1, (os::random() % num_ops_until_switch));
           chunks_moved = return_random_chunk_list_to_chunkmanager(list_length);
         }
       } else {
@@ -4346,7 +4346,7 @@
       num_ops_until_switch -= chunks_moved;
       if (chunks_moved == 0 || num_ops_until_switch <= 0) {
         return_phase = !return_phase;
-        num_ops_until_switch = MAX2(1, (int)(average_phase_length + os::random() % 8 - 4));
+        num_ops_until_switch = MAX2(1, (average_phase_length + os::random() % 8 - 4));
       }
     }
   }
--- a/hotspot/src/share/vm/prims/whitebox.cpp	Tue Jun 27 12:27:27 2017 +0000
+++ b/hotspot/src/share/vm/prims/whitebox.cpp	Wed Jun 28 16:14:20 2017 -0400
@@ -227,8 +227,8 @@
     return 3;
   }
 
-  long seed = os::random();
-  tty->print_cr("Random seed is %ld", seed);
+  int seed = os::random();
+  tty->print_cr("Random seed is %d", seed);
   os::init_random(seed);
 
   for (size_t i = 0; i < iterations; i++) {
--- a/hotspot/src/share/vm/runtime/os.cpp	Tue Jun 27 12:27:27 2017 +0000
+++ b/hotspot/src/share/vm/runtime/os.cpp	Wed Jun 28 16:14:20 2017 -0400
@@ -70,7 +70,7 @@
 address           os::_polling_page       = NULL;
 volatile int32_t* os::_mem_serialize_page = NULL;
 uintptr_t         os::_serialize_page_mask = 0;
-long              os::_rand_seed          = 1;
+volatile unsigned int os::_rand_seed      = 1;
 int               os::_processor_count    = 0;
 int               os::_initial_active_processor_count = 0;
 size_t            os::_page_sizes[os::page_sizes_max];
@@ -722,12 +722,12 @@
 #endif
 }
 
-void os::init_random(long initval) {
+void os::init_random(unsigned int initval) {
   _rand_seed = initval;
 }
 
 
-long os::random() {
+static int random_helper(unsigned int rand_seed) {
   /* standard, well-known linear congruential random generator with
    * next_rand = (16807*seed) mod (2**31-1)
    * see
@@ -736,14 +736,14 @@
    * (2) "Two Fast Implementations of the 'Minimal Standard' Random
    *     Number Generator", David G. Carta, Comm. ACM 33, 1 (Jan 1990), pp. 87-88.
   */
-  const long a = 16807;
-  const unsigned long m = 2147483647;
-  const long q = m / a;        assert(q == 127773, "weird math");
-  const long r = m % a;        assert(r == 2836, "weird math");
+  const unsigned int a = 16807;
+  const unsigned int m = 2147483647;
+  const int q = m / a;        assert(q == 127773, "weird math");
+  const int r = m % a;        assert(r == 2836, "weird math");
 
   // compute az=2^31p+q
-  unsigned long lo = a * (long)(_rand_seed & 0xFFFF);
-  unsigned long hi = a * (long)((unsigned long)_rand_seed >> 16);
+  unsigned int lo = a * (rand_seed & 0xFFFF);
+  unsigned int hi = a * (rand_seed >> 16);
   lo += (hi & 0x7FFF) << 16;
 
   // if q overflowed, ignore the overflow and increment q
@@ -758,7 +758,18 @@
     lo &= m;
     ++lo;
   }
-  return (_rand_seed = lo);
+  return lo;
+}
+
+int os::random() {
+  // Make updating the random seed thread safe.
+  while (true) {
+    unsigned int seed = _rand_seed;
+    int rand = random_helper(seed);
+    if (Atomic::cmpxchg(rand, &_rand_seed, seed) == seed) {
+      return rand;
+    }
+  }
 }
 
 // The INITIALIZED state is distinguished from the SUSPENDED state because the
@@ -1130,39 +1141,6 @@
 #endif
 }
 
-#ifdef ASSERT
-extern "C" void test_random() {
-  const double m = 2147483647;
-  double mean = 0.0, variance = 0.0, t;
-  long reps = 10000;
-  unsigned long seed = 1;
-
-  tty->print_cr("seed %ld for %ld repeats...", seed, reps);
-  os::init_random(seed);
-  long num;
-  for (int k = 0; k < reps; k++) {
-    num = os::random();
-    double u = (double)num / m;
-    assert(u >= 0.0 && u <= 1.0, "bad random number!");
-
-    // calculate mean and variance of the random sequence
-    mean += u;
-    variance += (u*u);
-  }
-  mean /= reps;
-  variance /= (reps - 1);
-
-  assert(num == 1043618065, "bad seed");
-  tty->print_cr("mean of the 1st 10000 numbers: %f", mean);
-  tty->print_cr("variance of the 1st 10000 numbers: %f", variance);
-  const double eps = 0.0001;
-  t = fabsd(mean - 0.5018);
-  assert(t < eps, "bad mean");
-  t = (variance - 0.3355) < 0.0 ? -(variance - 0.3355) : variance - 0.3355;
-  assert(t < eps, "bad variance");
-}
-#endif
-
 
 // Set up the boot classpath.
 
--- a/hotspot/src/share/vm/runtime/os.hpp	Tue Jun 27 12:27:27 2017 +0000
+++ b/hotspot/src/share/vm/runtime/os.hpp	Wed Jun 28 16:14:20 2017 -0400
@@ -743,8 +743,8 @@
   static int   sigexitnum_pd();
 
   // random number generation
-  static long random();                    // return 32bit pseudorandom number
-  static void init_random(long initval);   // initialize random sequence
+  static int random();                     // return 32bit pseudorandom number
+  static void init_random(unsigned int initval);    // initialize random sequence
 
   // Structured OS Exception support
   static void os_exception_wrapper(java_call_t f, JavaValue* value, const methodHandle& method, JavaCallArguments* args, Thread* thread);
@@ -956,7 +956,7 @@
 
 
  protected:
-  static long _rand_seed;                     // seed for random number generator
+  static volatile unsigned int _rand_seed;    // seed for random number generator
   static int _processor_count;                // number of processors
   static int _initial_active_processor_count; // number of active processors during initialization.
 
--- a/hotspot/src/share/vm/runtime/synchronizer.cpp	Tue Jun 27 12:27:27 2017 +0000
+++ b/hotspot/src/share/vm/runtime/synchronizer.cpp	Wed Jun 28 16:14:20 2017 -0400
@@ -672,8 +672,7 @@
 static inline intptr_t get_next_hash(Thread * Self, oop obj) {
   intptr_t value = 0;
   if (hashCode == 0) {
-    // This form uses an unguarded global Park-Miller RNG,
-    // so it's possible for two threads to race and generate the same RNG.
+    // This form uses global Park-Miller RNG.
     // On MP system we'll have lots of RW access to a global, so the
     // mechanism induces lots of coherency traffic.
     value = os::random();
--- a/hotspot/test/native/runtime/test_os.cpp	Tue Jun 27 12:27:27 2017 +0000
+++ b/hotspot/test/native/runtime/test_os.cpp	Wed Jun 28 16:14:20 2017 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, 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
@@ -108,6 +108,42 @@
   }
 }
 
+TEST(os, test_random) {
+  const double m = 2147483647;
+  double mean = 0.0, variance = 0.0, t;
+  const int reps = 10000;
+  unsigned int seed = 1;
+
+  // tty->print_cr("seed %ld for %ld repeats...", seed, reps);
+  os::init_random(seed);
+  int num;
+  for (int k = 0; k < reps; k++) {
+    num = os::random();
+    double u = (double)num / m;
+    ASSERT_TRUE(u >= 0.0 && u <= 1.0) << "bad random number!";
+
+    // calculate mean and variance of the random sequence
+    mean += u;
+    variance += (u*u);
+  }
+  mean /= reps;
+  variance /= (reps - 1);
+
+  ASSERT_EQ(num, 1043618065) << "bad seed";
+  // tty->print_cr("mean of the 1st 10000 numbers: %f", mean);
+  int intmean = mean*100;
+  ASSERT_EQ(intmean, 50);
+  // tty->print_cr("variance of the 1st 10000 numbers: %f", variance);
+  int intvariance = variance*100;
+  ASSERT_EQ(intvariance, 33);
+  const double eps = 0.0001;
+  t = fabsd(mean - 0.5018);
+  ASSERT_LT(t, eps) << "bad mean";
+  t = (variance - 0.3355) < 0.0 ? -(variance - 0.3355) : variance - 0.3355;
+  ASSERT_LT(t, eps) << "bad variance";
+}
+
+
 #ifdef ASSERT
 TEST_VM_ASSERT_MSG(os, page_size_for_region_with_zero_min_pages, "sanity") {
   size_t region_size = 16 * os::vm_page_size();