8143087: Miscellaneous changes imported from jsr166 CVS 2015-11
authordl
Wed, 25 Nov 2015 19:45:15 -0800
changeset 34348 ba5c2f2fc9d7
parent 34347 4a17f9e90a0f
child 34349 eddd54f8891e
8143087: Miscellaneous changes imported from jsr166 CVS 2015-11 Reviewed-by: martin, psandoz, chegar, shade, plevart
jdk/src/java.base/share/classes/java/util/concurrent/CountedCompleter.java
jdk/src/java.base/share/classes/java/util/concurrent/ScheduledThreadPoolExecutor.java
jdk/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
jdk/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java
jdk/src/java.base/share/classes/java/util/concurrent/locks/LockSupport.java
--- a/jdk/src/java.base/share/classes/java/util/concurrent/CountedCompleter.java	Wed Nov 25 18:56:44 2015 -0800
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/CountedCompleter.java	Wed Nov 25 19:45:15 2015 -0800
@@ -185,7 +185,7 @@
  *   }
  * }}</pre>
  *
- * As a further improvement, notice that the left task need not even exist.
+ * As a further optimization, notice that the left task need not even exist.
  * Instead of creating a new one, we can iterate using the original task,
  * and add a pending count for each fork.  Additionally, because no task
  * in this tree implements an {@link #onCompletion(CountedCompleter)} method,
@@ -208,7 +208,7 @@
  *   }
  * }}</pre>
  *
- * Additional improvements of such classes might entail precomputing
+ * Additional optimizations of such classes might entail precomputing
  * pending counts so that they can be established in constructors,
  * specializing classes for leaf steps, subdividing by say, four,
  * instead of two per iteration, and using an adaptive threshold
@@ -260,9 +260,9 @@
  * }}</pre>
  *
  * In this example, as well as others in which tasks have no other
- * effects except to compareAndSet a common result, the trailing
- * unconditional invocation of {@code tryComplete} could be made
- * conditional ({@code if (result.get() == null) tryComplete();})
+ * effects except to {@code compareAndSet} a common result, the
+ * trailing unconditional invocation of {@code tryComplete} could be
+ * made conditional ({@code if (result.get() == null) tryComplete();})
  * because no further bookkeeping is required to manage completions
  * once the root task completes.
  *
@@ -624,7 +624,7 @@
      * any one (versus all) of several subtask results are obtained.
      * However, in the common (and recommended) case in which {@code
      * setRawResult} is not overridden, this effect can be obtained
-     * more simply using {@code quietlyCompleteRoot();}.
+     * more simply using {@link #quietlyCompleteRoot()}.
      *
      * @param rawResult the raw result
      */
@@ -639,9 +639,9 @@
 
     /**
      * If this task's pending count is zero, returns this task;
-     * otherwise decrements its pending count and returns {@code
-     * null}. This method is designed to be used with {@link
-     * #nextComplete} in completion traversal loops.
+     * otherwise decrements its pending count and returns {@code null}.
+     * This method is designed to be used with {@link #nextComplete} in
+     * completion traversal loops.
      *
      * @return this task, if pending count was zero, else {@code null}
      */
--- a/jdk/src/java.base/share/classes/java/util/concurrent/ScheduledThreadPoolExecutor.java	Wed Nov 25 18:56:44 2015 -0800
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/ScheduledThreadPoolExecutor.java	Wed Nov 25 19:45:15 2015 -0800
@@ -132,11 +132,10 @@
     /*
      * This class specializes ThreadPoolExecutor implementation by
      *
-     * 1. Using a custom task type, ScheduledFutureTask for
-     *    tasks, even those that don't require scheduling (i.e.,
-     *    those submitted using ExecutorService execute, not
-     *    ScheduledExecutorService methods) which are treated as
-     *    delayed tasks with a delay of zero.
+     * 1. Using a custom task type ScheduledFutureTask, even for tasks
+     *    that don't require scheduling because they are submitted
+     *    using ExecutorService rather than ScheduledExecutorService
+     *    methods, which are treated as tasks with a delay of zero.
      *
      * 2. Using a custom queue (DelayedWorkQueue), a variant of
      *    unbounded DelayQueue. The lack of capacity constraint and
@@ -177,24 +176,17 @@
      */
     private static final AtomicLong sequencer = new AtomicLong();
 
-    /**
-     * Returns current nanosecond time.
-     */
-    static final long now() {
-        return System.nanoTime();
-    }
-
     private class ScheduledFutureTask<V>
             extends FutureTask<V> implements RunnableScheduledFuture<V> {
 
         /** Sequence number to break ties FIFO */
         private final long sequenceNumber;
 
-        /** The time the task is enabled to execute in nanoTime units */
+        /** The nanoTime-based time when the task is enabled to execute. */
         private volatile long time;
 
         /**
-         * Period in nanoseconds for repeating tasks.
+         * Period for repeating tasks, in nanoseconds.
          * A positive value indicates fixed-rate execution.
          * A negative value indicates fixed-delay execution.
          * A value of 0 indicates a non-repeating (one-shot) task.
@@ -244,7 +236,7 @@
         }
 
         public long getDelay(TimeUnit unit) {
-            return unit.convert(time - now(), NANOSECONDS);
+            return unit.convert(time - System.nanoTime(), NANOSECONDS);
         }
 
         public int compareTo(Delayed other) {
@@ -287,6 +279,9 @@
         }
 
         public boolean cancel(boolean mayInterruptIfRunning) {
+            // The racy read of heapIndex below is benign:
+            // if heapIndex < 0, then OOTA guarantees that we have surely
+            // been removed; else we recheck under lock in remove()
             boolean cancelled = super.cancel(mayInterruptIfRunning);
             if (cancelled && removeOnCancel && heapIndex >= 0)
                 remove(this);
@@ -528,7 +523,7 @@
      * Returns the nanoTime-based trigger time of a delayed action.
      */
     long triggerTime(long delay) {
-        return now() +
+        return System.nanoTime() +
             ((delay < (Long.MAX_VALUE >> 1)) ? delay : overflowFree(delay));
     }
 
--- a/jdk/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java	Wed Nov 25 18:56:44 2015 -0800
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java	Wed Nov 25 19:45:15 2015 -0800
@@ -922,7 +922,7 @@
 
     /**
      * Queries whether any threads have ever contended to acquire this
-     * synchronizer; that is if an acquire method has ever blocked.
+     * synchronizer; that is, if an acquire method has ever blocked.
      *
      * <p>In this implementation, this operation returns in
      * constant time.
@@ -977,13 +977,11 @@
          * guaranteeing termination.
          */
 
-        Node t = tail;
         Thread firstThread = null;
-        while (t != null && t != head) {
-            Thread tt = t.thread;
-            if (tt != null)
-                firstThread = tt;
-            t = t.prev;
+        for (Node p = tail; p != null && p != head; p = p.prev) {
+            Thread t = p.thread;
+            if (t != null)
+                firstThread = t;
         }
         return firstThread;
     }
@@ -1031,8 +1029,8 @@
      * <p>An invocation of this method is equivalent to (but may be
      * more efficient than):
      * <pre> {@code
-     * getFirstQueuedThread() != Thread.currentThread() &&
-     * hasQueuedThreads()}</pre>
+     * getFirstQueuedThread() != Thread.currentThread()
+     *   && hasQueuedThreads()}</pre>
      *
      * <p>Note that because cancellations due to interrupts and
      * timeouts may occur at any time, a {@code true} return does not
@@ -1635,7 +1633,7 @@
                     transferAfterCancelledWait(node);
                     break;
                 }
-                if (nanosTimeout >= SPIN_FOR_TIMEOUT_THRESHOLD)
+                if (nanosTimeout > SPIN_FOR_TIMEOUT_THRESHOLD)
                     LockSupport.parkNanos(this, nanosTimeout);
                 if ((interruptMode = checkInterruptWhileWaiting(node)) != 0)
                     break;
@@ -1723,7 +1721,7 @@
                     timedout = transferAfterCancelledWait(node);
                     break;
                 }
-                if (nanosTimeout >= SPIN_FOR_TIMEOUT_THRESHOLD)
+                if (nanosTimeout > SPIN_FOR_TIMEOUT_THRESHOLD)
                     LockSupport.parkNanos(this, nanosTimeout);
                 if ((interruptMode = checkInterruptWhileWaiting(node)) != 0)
                     break;
@@ -1847,8 +1845,9 @@
      * Initializes head and tail fields on first contention.
      */
     private final void initializeSyncQueue() {
-        if (U.compareAndSwapObject(this, HEAD, null, new Node()))
-            tail = head;
+        Node h;
+        if (U.compareAndSwapObject(this, HEAD, null, (h = new Node())))
+            tail = h;
     }
 
     /**
--- a/jdk/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java	Wed Nov 25 18:56:44 2015 -0800
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java	Wed Nov 25 19:45:15 2015 -0800
@@ -1388,7 +1388,7 @@
 
     /**
      * Queries whether any threads have ever contended to acquire this
-     * synchronizer; that is if an acquire method has ever blocked.
+     * synchronizer; that is, if an acquire method has ever blocked.
      *
      * <p>In this implementation, this operation returns in
      * constant time.
@@ -1443,13 +1443,11 @@
          * guaranteeing termination.
          */
 
-        Node t = tail;
         Thread firstThread = null;
-        while (t != null && t != head) {
-            Thread tt = t.thread;
-            if (tt != null)
-                firstThread = tt;
-            t = t.prev;
+        for (Node p = tail; p != null && p != head; p = p.prev) {
+            Thread t = p.thread;
+            if (t != null)
+                firstThread = t;
         }
         return firstThread;
     }
@@ -1497,8 +1495,8 @@
      * <p>An invocation of this method is equivalent to (but may be
      * more efficient than):
      * <pre> {@code
-     * getFirstQueuedThread() != Thread.currentThread() &&
-     * hasQueuedThreads()}</pre>
+     * getFirstQueuedThread() != Thread.currentThread()
+     *   && hasQueuedThreads()}</pre>
      *
      * <p>Note that because cancellations due to interrupts and
      * timeouts may occur at any time, a {@code true} return does not
@@ -2099,7 +2097,7 @@
                     transferAfterCancelledWait(node);
                     break;
                 }
-                if (nanosTimeout >= SPIN_FOR_TIMEOUT_THRESHOLD)
+                if (nanosTimeout > SPIN_FOR_TIMEOUT_THRESHOLD)
                     LockSupport.parkNanos(this, nanosTimeout);
                 if ((interruptMode = checkInterruptWhileWaiting(node)) != 0)
                     break;
@@ -2187,7 +2185,7 @@
                     timedout = transferAfterCancelledWait(node);
                     break;
                 }
-                if (nanosTimeout >= SPIN_FOR_TIMEOUT_THRESHOLD)
+                if (nanosTimeout > SPIN_FOR_TIMEOUT_THRESHOLD)
                     LockSupport.parkNanos(this, nanosTimeout);
                 if ((interruptMode = checkInterruptWhileWaiting(node)) != 0)
                     break;
@@ -2311,8 +2309,9 @@
      * Initializes head and tail fields on first contention.
      */
     private final void initializeSyncQueue() {
-        if (U.compareAndSwapObject(this, HEAD, null, new Node()))
-            tail = head;
+        Node h;
+        if (U.compareAndSwapObject(this, HEAD, null, (h = new Node())))
+            tail = h;
     }
 
     /**
--- a/jdk/src/java.base/share/classes/java/util/concurrent/locks/LockSupport.java	Wed Nov 25 18:56:44 2015 -0800
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/locks/LockSupport.java	Wed Nov 25 19:45:15 2015 -0800
@@ -81,12 +81,17 @@
  * method is designed for use only in constructions of the form:
  *
  * <pre> {@code
- * while (!canProceed()) { ... LockSupport.park(this); }}</pre>
+ * while (!canProceed()) {
+ *   // ensure request to unpark is visible to other threads
+ *   ...
+ *   LockSupport.park(this);
+ * }}</pre>
  *
- * where neither {@code canProceed} nor any other actions prior to the
- * call to {@code park} entail locking or blocking.  Because only one
- * permit is associated with each thread, any intermediary uses of
- * {@code park} could interfere with its intended effects.
+ * where no actions by the thread publishing a request to unpark,
+ * prior to the call to {@code park}, entail locking or blocking.
+ * Because only one permit is associated with each thread, any
+ * intermediary uses of {@code park}, including implicitly via class
+ * loading, could lead to an unresponsive thread (a "lost unpark").
  *
  * <p><b>Sample Usage.</b> Here is a sketch of a first-in-first-out
  * non-reentrant lock class:
@@ -98,26 +103,33 @@
  *
  *   public void lock() {
  *     boolean wasInterrupted = false;
- *     Thread current = Thread.currentThread();
- *     waiters.add(current);
+ *     // publish current thread for unparkers
+ *     waiters.add(Thread.currentThread());
  *
  *     // Block while not first in queue or cannot acquire lock
- *     while (waiters.peek() != current ||
+ *     while (waiters.peek() != Thread.currentThread() ||
  *            !locked.compareAndSet(false, true)) {
  *       LockSupport.park(this);
- *       if (Thread.interrupted()) // ignore interrupts while waiting
+ *       // ignore interrupts while waiting
+ *       if (Thread.interrupted())
  *         wasInterrupted = true;
  *     }
  *
  *     waiters.remove();
- *     if (wasInterrupted)          // reassert interrupt status on exit
- *       current.interrupt();
+ *     // ensure correct interrupt status on return
+ *     if (wasInterrupted)
+ *       Thread.currentThread().interrupt();
  *   }
  *
  *   public void unlock() {
  *     locked.set(false);
  *     LockSupport.unpark(waiters.peek());
  *   }
+ *
+ *   static {
+ *     // Reduce the risk of "lost unpark" due to classloading
+ *     Class<?> ensureLoaded = LockSupport.class;
+ *   }
  * }}</pre>
  */
 public class LockSupport {