Make Atomic*FieldUpdater fields static for better performance.

Compare cl/713006636. (Also, better document the similar code in `AbstractFuture`.) Note that I also evaluated performance with `VarHandle`, and I found it no better. (Maybe we did a similar experiment with `Unsafe` way back when and came to a similar conclusion?) Note that that's all based on _JVM_ performance (and on benchmarks that are not necessarily great). It's possible that Android it worth a further look someday. But our only option there _today_ might be `Unsafe`, and new usages of `Unsafe` would be moving backward. RELNOTES=n/a PiperOrigin-RevId: 724013501
google · Feb 6, 2025 · 7719744 · 7719744
1 parent a3500eb
commit 7719744
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 38 deletions.
diff --git a/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java b/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java
@@ -1485,19 +1485,34 @@ boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object updat
     }
   }
 
-  /** Returns an {@link AtomicReferenceFieldUpdater} for {@link #waiters}. */
+  /**
+   * Returns an {@link AtomicReferenceFieldUpdater} for {@link #waiters}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AbstractFuture}, as
+   * discussed in {@link #methodHandlesLookupFromWithinAbstractFuture}.
+   */
   private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter>
       waitersUpdaterFromWithinAbstractFuture() {
     return newUpdater(AbstractFuture.class, Waiter.class, "waiters");
   }
 
-  /** Returns an {@link AtomicReferenceFieldUpdater} for {@link #listeners}. */
+  /**
+   * Returns an {@link AtomicReferenceFieldUpdater} for {@link #listeners}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AbstractFuture}, as
+   * discussed in {@link #methodHandlesLookupFromWithinAbstractFuture}.
+   */
   private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener>
       listenersUpdaterFromWithinAbstractFuture() {
     return newUpdater(AbstractFuture.class, Listener.class, "listeners");
   }
 
-  /** Returns an {@link AtomicReferenceFieldUpdater} for {@link #value}. */
+  /**
+   * Returns an {@link AtomicReferenceFieldUpdater} for {@link #value}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AbstractFuture}, as
+   * discussed in {@link #methodHandlesLookupFromWithinAbstractFuture}.
+   */
   private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object>
       valueUpdaterFromWithinAbstractFuture() {
     return newUpdater(AbstractFuture.class, Object.class, "value");

diff --git a/android/guava/src/com/google/common/util/concurrent/AggregateFutureState.java b/android/guava/src/com/google/common/util/concurrent/AggregateFutureState.java
@@ -54,10 +54,7 @@ abstract class AggregateFutureState<OutputT extends @Nullable Object>
     AtomicHelper helper;
     Throwable thrownReflectionFailure = null;
     try {
-      helper =
-          new SafeAtomicHelper(
-              newUpdater(AggregateFutureState.class, Set.class, "seenExceptions"),
-              newUpdater(AggregateFutureState.class, "remaining"));
+      helper = new SafeAtomicHelper();
     } catch (Throwable reflectionFailure) { // sneaky checked exception
       // Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
       // getDeclaredField to throw a NoSuchFieldException when the field is definitely there.
@@ -156,20 +153,12 @@ abstract void compareAndSetSeenExceptions(
   }
 
   private static final class SafeAtomicHelper extends AtomicHelper {
-    final AtomicReferenceFieldUpdater<
+    private static final AtomicReferenceFieldUpdater<
             ? super AggregateFutureState<?>, ? super @Nullable Set<Throwable>>
-        seenExceptionsUpdater;
+        seenExceptionsUpdater = seenExceptionsUpdaterFromWithinAggregateFutureState();
 
-    final AtomicIntegerFieldUpdater<? super AggregateFutureState<?>> remainingCountUpdater;
-
-    SafeAtomicHelper(
-        AtomicReferenceFieldUpdater<
-                ? super AggregateFutureState<?>, ? super @Nullable Set<Throwable>>
-            seenExceptionsUpdater,
-        AtomicIntegerFieldUpdater<? super AggregateFutureState<?>> remainingCountUpdater) {
-      this.seenExceptionsUpdater = seenExceptionsUpdater;
-      this.remainingCountUpdater = remainingCountUpdater;
-    }
+    private static final AtomicIntegerFieldUpdater<? super AggregateFutureState<?>>
+        remainingCountUpdater = remainingCountUpdaterFromWithinAggregateFutureState();
 
     @Override
     void compareAndSetSeenExceptions(
@@ -201,4 +190,27 @@ int decrementAndGetRemainingCount(AggregateFutureState<?> state) {
       }
     }
   }
+
+  /**
+   * Returns an {@link AtomicReferenceFieldUpdater} for {@link #seenExceptions}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AggregateFutureState}, as
+   * discussed in {@link AbstractFuture#methodHandlesLookupFromWithinAbstractFuture}.
+   */
+  private static AtomicReferenceFieldUpdater<
+          ? super AggregateFutureState<?>, ? super @Nullable Set<Throwable>>
+      seenExceptionsUpdaterFromWithinAggregateFutureState() {
+    return newUpdater(AggregateFutureState.class, Set.class, "seenExceptions");
+  }
+
+  /**
+   * Returns an {@link AtomicIntegerFieldUpdater} for {@link #remaining}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AggregateFutureState}, as
+   * discussed in {@link AbstractFuture#methodHandlesLookupFromWithinAbstractFuture}.
+   */
+  private static AtomicIntegerFieldUpdater<? super AggregateFutureState<?>>
+      remainingCountUpdaterFromWithinAggregateFutureState() {
+    return newUpdater(AggregateFutureState.class, "remaining");
+  }
 }
diff --git a/guava/src/com/google/common/util/concurrent/AbstractFuture.java b/guava/src/com/google/common/util/concurrent/AbstractFuture.java
@@ -1597,19 +1597,34 @@ boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object updat
     }
   }
 
-  /** Returns an {@link AtomicReferenceFieldUpdater} for {@link #waiters}. */
+  /**
+   * Returns an {@link AtomicReferenceFieldUpdater} for {@link #waiters}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AbstractFuture}, as
+   * discussed in {@link #methodHandlesLookupFromWithinAbstractFuture}.
+   */
   private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter>
       waitersUpdaterFromWithinAbstractFuture() {
     return newUpdater(AbstractFuture.class, Waiter.class, "waiters");
   }
 
-  /** Returns an {@link AtomicReferenceFieldUpdater} for {@link #listeners}. */
+  /**
+   * Returns an {@link AtomicReferenceFieldUpdater} for {@link #listeners}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AbstractFuture}, as
+   * discussed in {@link #methodHandlesLookupFromWithinAbstractFuture}.
+   */
   private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener>
       listenersUpdaterFromWithinAbstractFuture() {
     return newUpdater(AbstractFuture.class, Listener.class, "listeners");
   }
 
-  /** Returns an {@link AtomicReferenceFieldUpdater} for {@link #value}. */
+  /**
+   * Returns an {@link AtomicReferenceFieldUpdater} for {@link #value}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AbstractFuture}, as
+   * discussed in {@link #methodHandlesLookupFromWithinAbstractFuture}.
+   */
   private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object>
       valueUpdaterFromWithinAbstractFuture() {
     return newUpdater(AbstractFuture.class, Object.class, "value");

diff --git a/guava/src/com/google/common/util/concurrent/AggregateFutureState.java b/guava/src/com/google/common/util/concurrent/AggregateFutureState.java
@@ -54,10 +54,7 @@ abstract class AggregateFutureState<OutputT extends @Nullable Object>
     AtomicHelper helper;
     Throwable thrownReflectionFailure = null;
     try {
-      helper =
-          new SafeAtomicHelper(
-              newUpdater(AggregateFutureState.class, Set.class, "seenExceptions"),
-              newUpdater(AggregateFutureState.class, "remaining"));
+      helper = new SafeAtomicHelper();
     } catch (Throwable reflectionFailure) { // sneaky checked exception
       // Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
       // getDeclaredField to throw a NoSuchFieldException when the field is definitely there.
@@ -156,20 +153,12 @@ abstract void compareAndSetSeenExceptions(
   }
 
   private static final class SafeAtomicHelper extends AtomicHelper {
-    final AtomicReferenceFieldUpdater<
+    private static final AtomicReferenceFieldUpdater<
             ? super AggregateFutureState<?>, ? super @Nullable Set<Throwable>>
-        seenExceptionsUpdater;
+        seenExceptionsUpdater = seenExceptionsUpdaterFromWithinAggregateFutureState();
 
-    final AtomicIntegerFieldUpdater<? super AggregateFutureState<?>> remainingCountUpdater;
-
-    SafeAtomicHelper(
-        AtomicReferenceFieldUpdater<
-                ? super AggregateFutureState<?>, ? super @Nullable Set<Throwable>>
-            seenExceptionsUpdater,
-        AtomicIntegerFieldUpdater<? super AggregateFutureState<?>> remainingCountUpdater) {
-      this.seenExceptionsUpdater = seenExceptionsUpdater;
-      this.remainingCountUpdater = remainingCountUpdater;
-    }
+    private static final AtomicIntegerFieldUpdater<? super AggregateFutureState<?>>
+        remainingCountUpdater = remainingCountUpdaterFromWithinAggregateFutureState();
 
     @Override
     void compareAndSetSeenExceptions(
@@ -201,4 +190,27 @@ int decrementAndGetRemainingCount(AggregateFutureState<?> state) {
       }
     }
   }
+
+  /**
+   * Returns an {@link AtomicReferenceFieldUpdater} for {@link #seenExceptions}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AggregateFutureState}, as
+   * discussed in {@link AbstractFuture#methodHandlesLookupFromWithinAbstractFuture}.
+   */
+  private static AtomicReferenceFieldUpdater<
+          ? super AggregateFutureState<?>, ? super @Nullable Set<Throwable>>
+      seenExceptionsUpdaterFromWithinAggregateFutureState() {
+    return newUpdater(AggregateFutureState.class, Set.class, "seenExceptions");
+  }
+
+  /**
+   * Returns an {@link AtomicIntegerFieldUpdater} for {@link #remaining}.
+   *
+   * <p>The creation of the updater has to happen directly inside {@link AggregateFutureState}, as
+   * discussed in {@link AbstractFuture#methodHandlesLookupFromWithinAbstractFuture}.
+   */
+  private static AtomicIntegerFieldUpdater<? super AggregateFutureState<?>>
+      remainingCountUpdaterFromWithinAggregateFutureState() {
+    return newUpdater(AggregateFutureState.class, "remaining");
+  }
 }