Skip to content

Commit

Permalink
Make AtomicReferenceFieldUpdater fields static for [better perfor…
Browse files Browse the repository at this point in the history 
…mance](https://shipilev.net/blog/2015/faster-atomic-fu/#:~:text=thrown%20out%20of%20the%20window).

This may eliminate the reason for [an `Unsafe`-based implementation](#6806) even under Java 8, but we will ideally confirm that with benchmarks before ripping that implementation out. (There's also Android: `AtomicReferenceFieldUpdater` is available there, but we should look into performance, including under older versions, especially with AFAIK no plans to remove `Unsafe` there.)

(Also, make a few other tiny edits to the backport copy so that it matches the mainline copy more closely.)

RELNOTES=n/a
PiperOrigin-RevId: 713006636
  • Loading branch information
@cpovirk
cpovirk authored and Google Java Core Libraries committed Jan 7, 2025
1 parent 12bf71e commit 400af25
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 63 deletions.
8 changes: 7 additions & 1 deletion .../test/com/google/common/util/concurrent/AggregateFutureStateFallbackAtomicHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,13 @@ public static TestSuite suite() {
// corresponding method on FuturesTest in the correct classloader.
TestSuite suite = new TestSuite(AggregateFutureStateFallbackAtomicHelperTest.class.getName());
for (Method method : FuturesTest.class.getDeclaredMethods()) {
if (Modifier.isPublic(method.getModifiers()) && method.getName().startsWith("test")) {
if (Modifier.isPublic(method.getModifiers())
&& method.getName().startsWith("test")
/*
* When we block access to AtomicReferenceFieldUpdater, we can't even reflect on
* AbstractFuture, since it declares methods that use that type in their signatures.
*/
&& !method.getName().equals("testFutures_nullChecks")) {
suite.addTest(
TestSuite.createTest(
AggregateFutureStateFallbackAtomicHelperTest.class, method.getName()));
Expand Down
65 changes: 33 additions & 32 deletions android/guava/src/com/google/common/util/concurrent/AbstractFuture.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,24 +159,15 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
helper = new UnsafeAtomicHelper();
} catch (Exception | Error unsafeFailure) { // sneaky checked exception
thrownUnsafeFailure = unsafeFailure;
// catch absolutely everything and fall through to our
// 'AtomicReferenceFieldUpdaterAtomicHelper' The access control checks that ARFU does means
// the caller class has to be AbstractFuture instead of
// AtomicReferenceFieldUpdaterAtomicHelper, so we annoyingly define these here
// Catch absolutely everything and fall through to AtomicReferenceFieldUpdaterAtomicHelper.
try {
helper =
new AtomicReferenceFieldUpdaterAtomicHelper(
newUpdater(Waiter.class, Thread.class, "thread"),
newUpdater(Waiter.class, Waiter.class, "next"),
newUpdater(AbstractFuture.class, Waiter.class, "waiters"),
newUpdater(AbstractFuture.class, Listener.class, "listeners"),
newUpdater(AbstractFuture.class, Object.class, "value"));
helper = new AtomicReferenceFieldUpdaterAtomicHelper();
} catch (Exception // sneaky checked exception
| Error atomicReferenceFieldUpdaterFailure) {
// 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.
// For these users fallback to a suboptimal implementation, based on synchronized. This will
// be a definite performance hit to those users.
// For these users fallback to a suboptimal implementation, based on synchronized. This
// will be a definite performance hit to those users.
thrownAtomicReferenceFieldUpdaterFailure = atomicReferenceFieldUpdaterFailure;
helper = new SynchronizedHelper();
}
Expand Down Expand Up @@ -716,7 +707,7 @@ mayInterruptIfRunning, new CancellationException("Future.cancel() was called."))
*
* <p>The default implementation does nothing.
*
* <p>This method is likely to be deprecated. Prefer to override {@link #afterDone}, consulting
* <p>This method is likely to be deprecated. Prefer to override {@link #afterDone}, checking
* {@link #wasInterrupted} to decide whether to interrupt your task.
*
* @since 10.0
Expand Down Expand Up @@ -1447,24 +1438,16 @@ boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object updat

/** {@link AtomicHelper} based on {@link AtomicReferenceFieldUpdater}. */
private static final class AtomicReferenceFieldUpdaterAtomicHelper extends AtomicHelper {
final AtomicReferenceFieldUpdater<Waiter, Thread> waiterThreadUpdater;
final AtomicReferenceFieldUpdater<Waiter, Waiter> waiterNextUpdater;
final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter> waitersUpdater;
final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener> listenersUpdater;
final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object> valueUpdater;

AtomicReferenceFieldUpdaterAtomicHelper(
AtomicReferenceFieldUpdater<Waiter, Thread> waiterThreadUpdater,
AtomicReferenceFieldUpdater<Waiter, Waiter> waiterNextUpdater,
AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter> waitersUpdater,
AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener> listenersUpdater,
AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object> valueUpdater) {
this.waiterThreadUpdater = waiterThreadUpdater;
this.waiterNextUpdater = waiterNextUpdater;
this.waitersUpdater = waitersUpdater;
this.listenersUpdater = listenersUpdater;
this.valueUpdater = valueUpdater;
}
private static final AtomicReferenceFieldUpdater<Waiter, Thread> waiterThreadUpdater =
newUpdater(Waiter.class, Thread.class, "thread");
private static final AtomicReferenceFieldUpdater<Waiter, Waiter> waiterNextUpdater =
newUpdater(Waiter.class, Waiter.class, "next");
private static final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter>
waitersUpdater = waitersUpdaterFromWithinAbstractFuture();
private static final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener>
listenersUpdater = listenersUpdaterFromWithinAbstractFuture();
private static final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object>
valueUpdater = valueUpdaterFromWithinAbstractFuture();

@Override
void putThread(Waiter waiter, Thread newValue) {
Expand Down Expand Up @@ -1502,6 +1485,24 @@ boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object updat
}
}

/** Returns an {@link AtomicReferenceFieldUpdater} for {@link #waiters}. */
private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter>
waitersUpdaterFromWithinAbstractFuture() {
return newUpdater(AbstractFuture.class, Waiter.class, "waiters");
}

/** Returns an {@link AtomicReferenceFieldUpdater} for {@link #listeners}. */
private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener>
listenersUpdaterFromWithinAbstractFuture() {
return newUpdater(AbstractFuture.class, Listener.class, "listeners");
}

/** Returns an {@link AtomicReferenceFieldUpdater} for {@link #value}. */
private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object>
valueUpdaterFromWithinAbstractFuture() {
return newUpdater(AbstractFuture.class, Object.class, "value");
}

/**
* {@link AtomicHelper} based on {@code synchronized} and volatile writes.
*
Expand Down
8 changes: 7 additions & 1 deletion .../test/com/google/common/util/concurrent/AggregateFutureStateFallbackAtomicHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,13 @@ public static TestSuite suite() {
// corresponding method on FuturesTest in the correct classloader.
TestSuite suite = new TestSuite(AggregateFutureStateFallbackAtomicHelperTest.class.getName());
for (Method method : FuturesTest.class.getDeclaredMethods()) {
if (Modifier.isPublic(method.getModifiers()) && method.getName().startsWith("test")) {
if (Modifier.isPublic(method.getModifiers())
&& method.getName().startsWith("test")
/*
* When we block access to AtomicReferenceFieldUpdater, we can't even reflect on
* AbstractFuture, since it declares methods that use that type in their signatures.
*/
&& !method.getName().equals("testFutures_nullChecks")) {
suite.addTest(
TestSuite.createTest(
AggregateFutureStateFallbackAtomicHelperTest.class, method.getName()));
Expand Down
59 changes: 30 additions & 29 deletions guava/src/com/google/common/util/concurrent/AbstractFuture.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,18 +186,9 @@ private static MethodHandles.Lookup methodHandlesLookupFromWithinAbstractFuture(
helper = new UnsafeAtomicHelper();
} catch (Exception | Error unsafeFailure) { // sneaky checked exception
thrownUnsafeFailure = unsafeFailure;
// catch absolutely everything and fall through to our
// 'AtomicReferenceFieldUpdaterAtomicHelper' The access control checks that ARFU does means
// the caller class has to be AbstractFuture instead of
// AtomicReferenceFieldUpdaterAtomicHelper, so we annoyingly define these here
// Catch absolutely everything and fall through to AtomicReferenceFieldUpdaterAtomicHelper.
try {
helper =
new AtomicReferenceFieldUpdaterAtomicHelper(
newUpdater(Waiter.class, Thread.class, "thread"),
newUpdater(Waiter.class, Waiter.class, "next"),
newUpdater(AbstractFuture.class, Waiter.class, "waiters"),
newUpdater(AbstractFuture.class, Listener.class, "listeners"),
newUpdater(AbstractFuture.class, Object.class, "value"));
helper = new AtomicReferenceFieldUpdaterAtomicHelper();
} catch (Exception // sneaky checked exception
| Error atomicReferenceFieldUpdaterFailure) {
// Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
Expand Down Expand Up @@ -1559,24 +1550,16 @@ boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object updat

/** {@link AtomicHelper} based on {@link AtomicReferenceFieldUpdater}. */
private static final class AtomicReferenceFieldUpdaterAtomicHelper extends AtomicHelper {
final AtomicReferenceFieldUpdater<Waiter, Thread> waiterThreadUpdater;
final AtomicReferenceFieldUpdater<Waiter, Waiter> waiterNextUpdater;
final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter> waitersUpdater;
final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener> listenersUpdater;
final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object> valueUpdater;

AtomicReferenceFieldUpdaterAtomicHelper(
AtomicReferenceFieldUpdater<Waiter, Thread> waiterThreadUpdater,
AtomicReferenceFieldUpdater<Waiter, Waiter> waiterNextUpdater,
AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter> waitersUpdater,
AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener> listenersUpdater,
AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object> valueUpdater) {
this.waiterThreadUpdater = waiterThreadUpdater;
this.waiterNextUpdater = waiterNextUpdater;
this.waitersUpdater = waitersUpdater;
this.listenersUpdater = listenersUpdater;
this.valueUpdater = valueUpdater;
}
private static final AtomicReferenceFieldUpdater<Waiter, Thread> waiterThreadUpdater =
newUpdater(Waiter.class, Thread.class, "thread");
private static final AtomicReferenceFieldUpdater<Waiter, Waiter> waiterNextUpdater =
newUpdater(Waiter.class, Waiter.class, "next");
private static final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter>
waitersUpdater = waitersUpdaterFromWithinAbstractFuture();
private static final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener>
listenersUpdater = listenersUpdaterFromWithinAbstractFuture();
private static final AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object>
valueUpdater = valueUpdaterFromWithinAbstractFuture();

@Override
void putThread(Waiter waiter, Thread newValue) {
Expand Down Expand Up @@ -1614,6 +1597,24 @@ boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object updat
}
}

/** Returns an {@link AtomicReferenceFieldUpdater} for {@link #waiters}. */
private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Waiter>
waitersUpdaterFromWithinAbstractFuture() {
return newUpdater(AbstractFuture.class, Waiter.class, "waiters");
}

/** Returns an {@link AtomicReferenceFieldUpdater} for {@link #listeners}. */
private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Listener>
listenersUpdaterFromWithinAbstractFuture() {
return newUpdater(AbstractFuture.class, Listener.class, "listeners");
}

/** Returns an {@link AtomicReferenceFieldUpdater} for {@link #value}. */
private static AtomicReferenceFieldUpdater<? super AbstractFuture<?>, Object>
valueUpdaterFromWithinAbstractFuture() {
return newUpdater(AbstractFuture.class, Object.class, "value");
}

/**
* {@link AtomicHelper} based on {@code synchronized} and volatile writes.
*
Expand Down

0 comments on commit 400af25

Please sign in to comment.