Use VarHandles to update shortest/longest duration 53/103153/4
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 8 Nov 2022 16:36:54 +0000 (17:36 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Tue, 8 Nov 2022 16:55:28 +0000 (17:55 +0100)
Rather than using ARFU's compareAndSet() and volatile reads, take
advantage of VarHandle's getAcquire() and compareAndExchangeRelease().
This makes things less ordered and more performant in face of races.
Also split out the update logic to aid inlining.

Change-Id: If60016b03c738240d7bdc041c32d5d908b3f002c
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
common/util/src/main/java/org/opendaylight/yangtools/util/ConcurrentDurationStatisticsTracker.java

index 085805af73b6598c254e50e453d91d16802fd16c..88c8b9ecea6baff04e09e0d4ae40b34ee5833849 100644 (file)
@@ -8,22 +8,29 @@
 package org.opendaylight.yangtools.util;
 
 import com.google.common.primitives.UnsignedLong;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
 import java.util.concurrent.atomic.AtomicLongFieldUpdater;
-import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 
 /**
  * Concurrent version of {@link DurationStatisticsTracker}.
  */
 // TODO: once DurationStatsTracker is gone make this class final
 class ConcurrentDurationStatisticsTracker extends DurationStatisticsTracker {
+    private static final VarHandle LONGEST;
+    private static final VarHandle SHORTEST;
 
-    private static final AtomicReferenceFieldUpdater<ConcurrentDurationStatisticsTracker, DurationWithTime>
-        LONGEST_UPDATER = AtomicReferenceFieldUpdater.newUpdater(ConcurrentDurationStatisticsTracker.class,
-                DurationWithTime.class, "longest");
-
-    private static final AtomicReferenceFieldUpdater<ConcurrentDurationStatisticsTracker, DurationWithTime>
-        SHORTEST_UPDATER = AtomicReferenceFieldUpdater.newUpdater(ConcurrentDurationStatisticsTracker.class,
-                DurationWithTime.class, "shortest");
+    static {
+        final var lookup = MethodHandles.lookup();
+        try {
+            LONGEST = lookup.findVarHandle(
+                ConcurrentDurationStatisticsTracker.class, "longest", DurationWithTime.class);
+            SHORTEST = lookup.findVarHandle(
+                ConcurrentDurationStatisticsTracker.class, "shortest", DurationWithTime.class);
+        } catch (NoSuchFieldException | IllegalAccessException e) {
+            throw new ExceptionInInitializerError(e);
+        }
+    }
 
     private static final AtomicLongFieldUpdater<ConcurrentDurationStatisticsTracker> COUNT_UPDATER =
             AtomicLongFieldUpdater.newUpdater(ConcurrentDurationStatisticsTracker.class, "count");
@@ -32,8 +39,8 @@ class ConcurrentDurationStatisticsTracker extends DurationStatisticsTracker {
 
     private volatile long sum = 0;
     private volatile long count = 0;
-    private volatile DurationWithTime longest = null;
-    private volatile DurationWithTime shortest = null;
+    private volatile DurationWithTime longest;
+    private volatile DurationWithTime shortest;
 
     ConcurrentDurationStatisticsTracker() {
         // Hidden on purpose
@@ -46,34 +53,45 @@ class ConcurrentDurationStatisticsTracker extends DurationStatisticsTracker {
         COUNT_UPDATER.incrementAndGet(this);
 
         /*
-         * Now the hairy 'min/max' things. The notion of "now" we cache,
-         * so the first time we use it, we do not call it twice. We populate
-         * it lazily, though.
+         * Now the hairy 'min/max' things. The notion of "now" we cache, so the first time we use it, we do not call it
+         * twice. We populate it lazily, though.
          *
-         * The longest/shortest stats both are encapsulated in an object,
-         * so we update them atomically and we minimize the number of volatile
-         * operations.
+         * The longest/shortest stats both are encapsulated in an object, so we update them atomically and we minimize
+         * the number of volatile operations.
          */
-        DurationWithTime current = shortest;
-        if (current == null || duration < current.getDuration()) {
-            final DurationWithTime newObj = new DurationWithTime(duration, System.currentTimeMillis());
-            while (!SHORTEST_UPDATER.weakCompareAndSet(this, current, newObj)) {
-                current = shortest;
-                if (current != null && duration >= current.getDuration()) {
-                    break;
-                }
+        final var currentShortest = (DurationWithTime) SHORTEST.getAcquire(this);
+        if (currentShortest == null || duration < currentShortest.getDuration()) {
+            updateShortest(currentShortest, duration);
+        }
+        final var currentLongest = (DurationWithTime) LONGEST.getAcquire(this);
+        if (currentLongest == null || duration > currentLongest.getDuration()) {
+            updateLongest(currentLongest, duration);
+        }
+    }
+
+    private void updateShortest(final DurationWithTime prev, final long duration) {
+        final var newObj = new DurationWithTime(duration, System.currentTimeMillis());
+
+        var expected = prev;
+        while (true) {
+            final var witness = (DurationWithTime) SHORTEST.compareAndExchangeRelease(this, expected, newObj);
+            if (witness == expected || duration >= witness.getDuration()) {
+                break;
             }
+            expected = witness;
         }
+    }
+
+    private void updateLongest(final DurationWithTime prev, final long duration) {
+        final var newObj = new DurationWithTime(duration, System.currentTimeMillis());
 
-        current = longest;
-        if (current == null || duration > current.getDuration()) {
-            final DurationWithTime newObj = new DurationWithTime(duration, System.currentTimeMillis());
-            while (!LONGEST_UPDATER.weakCompareAndSet(this, current, newObj)) {
-                current = longest;
-                if (current != null && duration <= current.getDuration()) {
-                    break;
-                }
+        var expected = prev;
+        while (true) {
+            final var witness = (DurationWithTime) LONGEST.compareAndExchangeRelease(this, expected, newObj);
+            if (witness == expected || duration <= witness.getDuration()) {
+                break;
             }
+            expected = witness;
         }
     }