8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity

Reviewed-by: dholmes, alanb

src/hotspot/os/posix/os_posix.cpp

@@ -1538,6 +1538,12 @@ void PlatformEvent::park() {       // AKA "down()"
 }
 
 int PlatformEvent::park(jlong millis) {
+  return park_nanos(millis_to_nanos_bounded(millis));
+}
+
+int PlatformEvent::park_nanos(jlong nanos) {
+  assert(nanos > 0, "nanos are positive");
+
   // Transitions for _event:
   //   -1 => -1 : illegal
   //    1 =>  0 : pass - return immediately
@@ -1557,7 +1563,7 @@ int PlatformEvent::park(jlong millis) {
 
   if (v == 0) { // Do this the hard way by blocking ...
     struct timespec abst;
-    to_abstime(&abst, millis_to_nanos_bounded(millis), false, false);
+    to_abstime(&abst, nanos, false, false);
 
     int ret = OS_TIMEOUT;
     int status = pthread_mutex_lock(_mutex);

src/hotspot/os/posix/park_posix.hpp

@@ -54,6 +54,7 @@ class PlatformEvent : public CHeapObj<mtSynchronizer> {
   PlatformEvent();
   void park();
   int  park(jlong millis);
+  int  park_nanos(jlong nanos);
   void unpark();
 
   // Use caution with reset() and fired() -- they may require MEMBARs

src/hotspot/os/windows/os_windows.cpp

@@ -5249,6 +5249,21 @@ class HighResolutionInterval : public CHeapObj<mtThread> {
 // explicit "PARKED" == 01b and "SIGNALED" == 10b bits.
 //
 
+int PlatformEvent::park_nanos(jlong nanos) {
+  assert(nanos > 0, "nanos are positive");
+
+  // Windows timers are still quite unpredictable to handle sub-millisecond granularity.
+  // Instead of implementing sub-millisecond sleeps, fall back to the usual behavior of
+  // rounding up any excess requested nanos to the full millisecond. This is how
+  // Thread.sleep(millis, nanos) has always behaved with only millisecond granularity.
+  jlong millis = nanos / NANOSECS_PER_MILLISEC;
+  if (nanos > millis * NANOSECS_PER_MILLISEC) {
+    millis++;
+  }
+  assert(millis > 0, "should always be positive");
+  return park(millis);
+}
+
 int PlatformEvent::park(jlong Millis) {
   // Transitions for _Event:
   //   -1 => -1 : illegal

src/hotspot/os/windows/park_windows.hpp

@@ -48,9 +48,10 @@ class PlatformEvent : public CHeapObj<mtSynchronizer> {
     // Exercise caution using reset() and fired() - they may require MEMBARs
     void reset() { _Event = 0 ; }
     int  fired() { return _Event; }
-    void park () ;
+    void park();
-    void unpark () ;
+    void unpark();
-    int  park (jlong millis) ;
+    int  park(jlong millis);
+    int  park_nanos(jlong nanos);
 };
 
 class PlatformParker {

src/hotspot/share/include/jvm.h

@@ -276,7 +276,7 @@ JNIEXPORT void JNICALL
 JVM_Yield(JNIEnv *env, jclass threadClass);
 
 JNIEXPORT void JNICALL
-JVM_Sleep(JNIEnv *env, jclass threadClass, jlong millis);
+JVM_Sleep(JNIEnv *env, jclass threadClass, jlong nanos);
 
 JNIEXPORT jobject JNICALL
 JVM_CurrentCarrierThread(JNIEnv *env, jclass threadClass);

src/hotspot/share/prims/jvm.cpp

@@ -3044,9 +3044,9 @@ JVM_LEAF(void, JVM_Yield(JNIEnv *env, jclass threadClass))
   os::naked_yield();
 JVM_END
 
-JVM_ENTRY(void, JVM_Sleep(JNIEnv* env, jclass threadClass, jlong millis))
+JVM_ENTRY(void, JVM_Sleep(JNIEnv* env, jclass threadClass, jlong nanos))
-  if (millis < 0) {
+  if (nanos < 0) {
-    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "timeout value is negative");
+    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "nanosecond timeout value out of range");
   }
 
   if (thread->is_interrupted(true) && !HAS_PENDING_EXCEPTION) {
@@ -3057,14 +3057,14 @@ JVM_ENTRY(void, JVM_Sleep(JNIEnv* env, jclass threadClass, jlong millis))
   // And set new thread state to SLEEPING.
   JavaThreadSleepState jtss(thread);
 
-  HOTSPOT_THREAD_SLEEP_BEGIN(millis);
+  HOTSPOT_THREAD_SLEEP_BEGIN(nanos / NANOSECS_PER_MILLISEC);
 
-  if (millis == 0) {
+  if (nanos == 0) {
     os::naked_yield();
   } else {
     ThreadState old_state = thread->osthread()->get_state();
     thread->osthread()->set_state(SLEEPING);
-    if (!thread->sleep(millis)) { // interrupted
+    if (!thread->sleep_nanos(nanos)) { // interrupted
       // An asynchronous exception could have been thrown on
       // us while we were sleeping. We do not overwrite those.
       if (!HAS_PENDING_EXCEPTION) {

src/hotspot/share/runtime/javaThread.cpp

@@ -1982,11 +1982,24 @@ Klass* JavaThread::security_get_caller_class(int depth) {
   return nullptr;
 }
 
+// Internal convenience function for millisecond resolution sleeps.
+bool JavaThread::sleep(jlong millis) {
+  jlong nanos;
+  if (millis > max_jlong / NANOUNITS_PER_MILLIUNIT) {
+    // Conversion to nanos would overflow, saturate at max
+    nanos = max_jlong;
+  } else {
+    nanos = millis * NANOUNITS_PER_MILLIUNIT;
+  }
+  return sleep_nanos(nanos);
+}
+
 // java.lang.Thread.sleep support
 // Returns true if sleep time elapsed as expected, and false
 // if the thread was interrupted.
-bool JavaThread::sleep(jlong millis) {
+bool JavaThread::sleep_nanos(jlong nanos) {
   assert(this == Thread::current(),  "thread consistency check");
+  assert(nanos >= 0, "nanos are in range");
 
   ParkEvent * const slp = this->_SleepEvent;
   // Because there can be races with thread interruption sending an unpark()
@@ -2000,20 +2013,22 @@ bool JavaThread::sleep(jlong millis) {
 
   jlong prevtime = os::javaTimeNanos();
 
+  jlong nanos_remaining = nanos;
+
   for (;;) {
     // interruption has precedence over timing out
     if (this->is_interrupted(true)) {
       return false;
     }
 
-    if (millis <= 0) {
+    if (nanos_remaining <= 0) {
       return true;
     }
 
     {
       ThreadBlockInVM tbivm(this);
       OSThreadWaitState osts(this->osthread(), false /* not Object.wait() */);
-      slp->park(millis);
+      slp->park_nanos(nanos_remaining);
     }
 
     // Update elapsed time tracking
@@ -2024,7 +2039,7 @@ bool JavaThread::sleep(jlong millis) {
       assert(false,
              "unexpected time moving backwards detected in JavaThread::sleep()");
     } else {
-      millis -= (newtime - prevtime) / NANOSECS_PER_MILLISEC;
+      nanos_remaining -= (newtime - prevtime);
     }
     prevtime = newtime;
   }

src/hotspot/share/runtime/javaThread.hpp

@@ -1140,6 +1140,7 @@ private:
   ParkEvent * _SleepEvent;
 public:
   bool sleep(jlong millis);
+  bool sleep_nanos(jlong nanos);
 
   // java.lang.Thread interruption support
   void interrupt();

src/java.base/share/classes/java/lang/Thread.java

@@ -506,14 +506,14 @@ public class Thread implements Runnable {
             if (currentThread() instanceof VirtualThread vthread) {
                 vthread.sleepNanos(nanos);
             } else {
-                sleep0(millis);
+                sleep0(nanos);
             }
         } finally {
             afterSleep(event);
         }
     }
 
-    private static native void sleep0(long millis) throws InterruptedException;
+    private static native void sleep0(long nanos) throws InterruptedException;
 
     /**
      * Causes the currently executing thread to sleep (temporarily cease
@@ -555,11 +555,7 @@ public class Thread implements Runnable {
             if (currentThread() instanceof VirtualThread vthread) {
                 vthread.sleepNanos(totalNanos);
             } else {
-                // millisecond precision
+                sleep0(totalNanos);
-                if (nanos > 0 && millis < Long.MAX_VALUE) {
-                    millis++;
-                }
-                sleep0(millis);
             }
         } finally {
             afterSleep(event);
@@ -593,12 +589,7 @@ public class Thread implements Runnable {
             if (currentThread() instanceof VirtualThread vthread) {
                 vthread.sleepNanos(nanos);
             } else {
-                // millisecond precision
+                sleep0(nanos);
-                long millis = NANOSECONDS.toMillis(nanos);
-                if (nanos > MILLISECONDS.toNanos(millis)) {
-                    millis += 1L;
-                }
-                sleep0(millis);
             }
         } finally {
             afterSleep(event);

test/jdk/java/lang/Thread/SleepSanity.java

@@ -0,0 +1,119 @@
+/*
+ * Copyright Amazon.com Inc. 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @summary Sanity test Thread.sleep behavior
+ * @run junit SleepSanity
+ */
+
+import java.util.concurrent.TimeUnit;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
+public class SleepSanity {
+
+    static final int[] TRY_MILLIS = new int[] { 0, 1, 10, 100, 1_000 };
+    static final int[] TRY_NANOS  = new int[] { 0, 1, 10, 100, 1_000, 10_000, 100_000, 999_999 };
+
+    @Test
+    void testMillis() throws Exception {
+        testIAE(() -> Thread.sleep(-1), "timeout value is negative");
+
+        testTimeout(() -> Thread.sleep(10_000),            5_000);
+        testTimeout(() -> Thread.sleep(Integer.MAX_VALUE), 5_000);
+        testTimeout(() -> Thread.sleep(Long.MAX_VALUE),    5_000);
+
+        for (final int millis : TRY_MILLIS) {
+            testTimes(() -> Thread.sleep(millis), millis, 20_000);
+        }
+    }
+
+    @Test
+    void testMillisNanos() throws Exception {
+        testIAE(() -> Thread.sleep(-1),    "timeout value is negative");
+
+        testIAE(() -> Thread.sleep(0, -1),                "nanosecond timeout value out of range");
+        testIAE(() -> Thread.sleep(0, 1_000_000),         "nanosecond timeout value out of range");
+        testIAE(() -> Thread.sleep(0, Integer.MAX_VALUE), "nanosecond timeout value out of range");
+
+        testTimeout(() -> Thread.sleep(10_000, 0),            5_000);
+        testTimeout(() -> Thread.sleep(Integer.MAX_VALUE, 0), 5_000);
+        testTimeout(() -> Thread.sleep(Long.MAX_VALUE, 0),    5_000);
+
+        testTimeout(() -> Thread.sleep(10_000, 999_999),            5_000);
+        testTimeout(() -> Thread.sleep(Integer.MAX_VALUE, 999_999), 5_000);
+        testTimeout(() -> Thread.sleep(Long.MAX_VALUE, 999_999),    5_000);
+
+        for (final int millis : TRY_MILLIS) {
+            for (final int nanos : TRY_NANOS) {
+                testTimes(() -> Thread.sleep(millis, nanos), millis, 20_000);
+            }
+        }
+    }
+
+    private static void testTimes(TestCase t, long millisMin, long millisMax) throws Exception {
+        long start = System.nanoTime();
+        t.run();
+        long end = System.nanoTime();
+        long duration = TimeUnit.NANOSECONDS.toMillis(end - start);
+        assertTrue(duration >= millisMin, "Duration " + duration + "ms, expected >= " + millisMin + "ms");
+        assertTrue(duration <= millisMax, "Duration " + duration + "ms, expected <= " + millisMax + "ms");
+    }
+
+    private static void testTimeout(TestCase t, long millis) throws Exception {
+        Thread captThread = Thread.currentThread();
+        Thread watcher = new Thread(() -> {
+            try {
+                Thread.sleep(millis);
+            } catch (InterruptedException ie)  {
+                // Do nothing
+            }
+            captThread.interrupt();
+        });
+        watcher.setDaemon(true);
+        watcher.start();
+        try {
+            t.run();
+            fail("Exited before timeout");
+        } catch (InterruptedException ie) {
+            // Expected
+        }
+        watcher.join();
+    }
+
+    private static void testIAE(TestCase t, String msg) throws Exception {
+        try {
+            t.run();
+            fail("Should have thrown the IAE");
+        } catch (IllegalArgumentException iae) {
+            assertTrue(iae.getMessage().contains(msg),
+                       "Thrown IAE does not contain the string: " + msg + " " + iae);
+        }
+    }
+
+    private interface TestCase {
+        void run() throws Exception;
+    }
+
+}

test/micro/org/openjdk/bench/java/lang/ThreadSleep.java

@@ -0,0 +1,68 @@
+/*
+ * Copyright Amazon.com Inc. 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.bench.java.lang;
+
+import org.openjdk.jmh.annotations.*;
+import java.util.concurrent.TimeUnit;
+
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Fork(3)
+@State(Scope.Benchmark)
+public class ThreadSleep {
+
+    @Param({"0",
+            "1",
+            "10",
+            "100",
+            "1000",
+            "10000",
+            "100000",
+            "1000000",
+            "10000000",
+            "100000000",
+            "1000000000"})
+    private int sleep;
+
+    private long millis;
+    private int nanos;
+
+    @Setup
+    public void setup() {
+        millis = TimeUnit.NANOSECONDS.toMillis(sleep);
+        nanos = (int)(sleep - TimeUnit.MILLISECONDS.toNanos(millis));
+    }
+
+    @Benchmark
+    public void millis() throws InterruptedException {
+        Thread.sleep(millis);
+    }
+
+    @Benchmark
+    public void millisNanos() throws InterruptedException {
+        Thread.sleep(millis, nanos);
+    }
+
+}