Use nanosecond precision for Kernel#sleep and Mutex#sleep

* Fixes oracle#2716
thermofisher-jch · Aug 16, 2023 · 8684a57 · 8684a57
1 parent 06187ab
commit 8684a57
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ Bug fixes:
 * The `strip` option `--keep-section=.llvmbc` is not supported on macOS (#2697, @eregon).
 * Disallow the marshaling of polyglot exceptions since we can't properly reconstruct them (@nirvdrum).
 * Fix `String#split` missing a value in its return array when called with a pattern of `" "` and a _limit_ value > 0 on a string with trailing whitespace where the limit hasn't been met (@nirvdrum).
+* Fix `Kernel#sleep` and `Mutex#sleep` for durations smaller than 1 millisecond (#2716, @eregon).
 
 Compatibility:
 

diff --git a/...core/cast/DurationToMillisecondsNode.java → .../core/cast/DurationToNanoSecondsNode.java b/...core/cast/DurationToMillisecondsNode.java → .../core/cast/DurationToNanoSecondsNode.java
@@ -21,13 +21,15 @@
 import com.oracle.truffle.api.profiles.ConditionProfile;
 import org.truffleruby.language.dispatch.DispatchNode;
 
+import java.util.concurrent.TimeUnit;
+
 @NodeChild(value = "duration", type = RubyBaseNodeWithExecute.class)
-public abstract class DurationToMillisecondsNode extends RubyBaseNodeWithExecute {
+public abstract class DurationToNanoSecondsNode extends RubyBaseNodeWithExecute {
 
     private final ConditionProfile durationLessThanZeroProfile = ConditionProfile.create();
     private final boolean acceptsNil;
 
-    public DurationToMillisecondsNode(boolean acceptsNil) {
+    public DurationToNanoSecondsNode(boolean acceptsNil) {
         this.acceptsNil = acceptsNil;
     }
 
@@ -36,19 +38,14 @@ protected long noDuration(NotProvided duration) {
         return Long.MAX_VALUE;
     }
 
-    @Specialization
-    protected long duration(int duration) {
-        return validate(duration * 1000L);
-    }
-
     @Specialization
     protected long duration(long duration) {
-        return validate(duration * 1000);
+        return validate(TimeUnit.SECONDS.toNanos(duration));
     }
 
     @Specialization
     protected long duration(double duration) {
-        return validate((long) (duration * 1000));
+        return validate((long) (duration * 1e9));
     }
 
     @Specialization
@@ -64,20 +61,20 @@ protected long durationNil(Nil duration) {
 
     @Specialization
     protected Object duration(RubyDynamicObject duration,
-            @Cached DispatchNode durationToMilliseconds,
+            @Cached DispatchNode durationToNanoSeconds,
             @Cached ToLongNode toLongNode) {
-        final Object milliseconds = durationToMilliseconds.call(
+        final Object nanoseconds = durationToNanoSeconds.call(
                 coreLibrary().truffleKernelOperationsModule,
-                "convert_duration_to_milliseconds",
+                "convert_duration_to_nanoseconds",
                 duration);
-        return validate(toLongNode.execute(milliseconds));
+        return validate(toLongNode.execute(nanoseconds));
     }
 
-    private long validate(long durationInMillis) {
-        if (durationLessThanZeroProfile.profile(durationInMillis < 0)) {
+    private long validate(long durationInNanos) {
+        if (durationLessThanZeroProfile.profile(durationInNanos < 0)) {
             throw new RaiseException(getContext(), coreExceptions().argumentErrorTimeIntervalPositive(this));
         }
-        return durationInMillis;
+        return durationInNanos;
     }
 
 }
diff --git a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java
@@ -44,7 +44,7 @@
 import org.truffleruby.core.cast.BooleanCastNode;
 import org.truffleruby.core.cast.BooleanCastNodeGen;
 import org.truffleruby.core.cast.BooleanCastWithDefaultNode;
-import org.truffleruby.core.cast.DurationToMillisecondsNodeGen;
+import org.truffleruby.core.cast.DurationToNanoSecondsNodeGen;
 import org.truffleruby.core.cast.NameToJavaStringNode;
 import org.truffleruby.core.cast.ToIntNode;
 import org.truffleruby.core.cast.ToStrNode;
@@ -1610,13 +1610,13 @@ public abstract static class SleepNode extends CoreMethodNode {
 
         @CreateCast("duration")
         protected RubyBaseNodeWithExecute coerceDuration(RubyBaseNodeWithExecute duration) {
-            return DurationToMillisecondsNodeGen.create(false, duration);
+            return DurationToNanoSecondsNodeGen.create(false, duration);
         }
 
         @Specialization
-        protected long sleep(long durationInMillis,
+        protected long sleep(long durationInNanos,
                 @Cached BranchProfile errorProfile) {
-            if (durationInMillis < 0) {
+            if (durationInNanos < 0) {
                 errorProfile.enter();
                 throw new RaiseException(
                         getContext(),
@@ -1629,27 +1629,26 @@ protected long sleep(long durationInMillis,
             // it should only be considered if we are inside the sleep when Thread#{run,wakeup} is called.
             thread.wakeUp.set(false);
 
-            return sleepFor(getContext(), thread, durationInMillis, this);
+            return sleepFor(getContext(), thread, durationInNanos, this);
         }
 
         @TruffleBoundary
-        public static long sleepFor(RubyContext context, RubyThread thread, long durationInMillis,
+        public static long sleepFor(RubyContext context, RubyThread thread, long durationInNanos,
                 Node currentNode) {
-            assert durationInMillis >= 0;
+            assert durationInNanos >= 0;
 
             // We want a monotonic clock to measure sleep duration
             final long startInNanos = System.nanoTime();
 
             context.getThreadManager().runUntilResult(currentNode, () -> {
                 final long nowInNanos = System.nanoTime();
                 final long sleptInNanos = nowInNanos - startInNanos;
-                final long sleptInMillis = TimeUnit.NANOSECONDS.toMillis(sleptInNanos);
 
-                if (sleptInMillis >= durationInMillis || thread.wakeUp.getAndSet(false)) {
+                if (sleptInNanos >= durationInNanos || thread.wakeUp.getAndSet(false)) {
                     return BlockingAction.SUCCESS;
                 }
 
-                Thread.sleep(durationInMillis - sleptInMillis);
+                TimeUnit.NANOSECONDS.sleep(durationInNanos - sleptInNanos);
                 return BlockingAction.SUCCESS;
             });
 

diff --git a/src/main/java/org/truffleruby/core/mutex/MutexNodes.java b/src/main/java/org/truffleruby/core/mutex/MutexNodes.java
@@ -21,7 +21,7 @@
 import org.truffleruby.builtins.CoreModule;
 import org.truffleruby.builtins.UnaryCoreMethodNode;
 import org.truffleruby.builtins.YieldingCoreMethodNode;
-import org.truffleruby.core.cast.DurationToMillisecondsNodeGen;
+import org.truffleruby.core.cast.DurationToNanoSecondsNodeGen;
 import org.truffleruby.core.kernel.KernelNodes;
 import org.truffleruby.core.klass.RubyClass;
 import org.truffleruby.core.proc.RubyProc;
@@ -155,11 +155,11 @@ public abstract static class SleepNode extends CoreMethodNode {
 
         @CreateCast("duration")
         protected RubyBaseNodeWithExecute coerceDuration(RubyBaseNodeWithExecute duration) {
-            return DurationToMillisecondsNodeGen.create(true, duration);
+            return DurationToNanoSecondsNodeGen.create(true, duration);
         }
 
         @Specialization
-        protected long sleep(RubyMutex mutex, long durationInMillis,
+        protected long sleep(RubyMutex mutex, long durationInNanos,
                 @Cached BranchProfile errorProfile) {
             final ReentrantLock lock = mutex.lock;
             final RubyThread thread = getLanguage().getCurrentThread();
@@ -175,7 +175,7 @@ protected long sleep(RubyMutex mutex, long durationInMillis,
 
             MutexOperations.unlock(lock, thread);
             try {
-                return KernelNodes.SleepNode.sleepFor(getContext(), thread, durationInMillis, this);
+                return KernelNodes.SleepNode.sleepFor(getContext(), thread, durationInNanos, this);
             } finally {
                 MutexOperations.lockEvenWithExceptions(getContext(), lock, thread, this);
             }

diff --git a/src/main/ruby/truffleruby/core/truffle/kernel_operations.rb b/src/main/ruby/truffleruby/core/truffle/kernel_operations.rb
@@ -22,12 +22,12 @@ def self.to_enum_with_size(enum, method, size_method)
       enum.to_enum(method) { enum.send(size_method) }
     end
 
-    def self.convert_duration_to_milliseconds(duration)
+    def self.convert_duration_to_nanoseconds(duration)
       unless duration.respond_to?(:divmod)
         raise TypeError, "can't convert #{duration.class} into time interval"
       end
       a, b = duration.divmod(1)
-      ((a + b) * 1000)
+      ((a + b) * 1_000_000_000)
     end
 
     def self.define_hooked_variable(name, getter, setter, defined = proc { 'global-variable' })