Skip to content

Commit

Permalink
Make AbstractFuture use VarHandle when available.
Browse files Browse the repository at this point in the history
For now, this is only for the JRE flavor, not for Android.

Our not entirely great benchmarks suggest that this may actually improve performance slightly over the current `Unsafe`-based implementation. This matches my sense that [alternatives to `Unsafe` have gotten faster](#6806 (comment)).

There are plenty of other [places in Guava that we use `Unsafe`](#6806), but this is a start.

Also: I'm going to consider this CL to "fix" #6549, even though:
- We already started requiring newer versions of Java to build our _tests_ in cl/705512728. (This CL is the first to require a newer JDK to build _prod_ code, again only to _build_ it, not to _run_ it.)
- We already started requiring newer versions of Java to build our _GWT_ module in cl/711487270.
- This CL requires only Java 9, not Java 11.
- None of the changes so far should require people who _build our Maven project_ to do anything (aside from GWT users), since our build automatically downloads a new JDK to use for javac since cl/655647768.
RELNOTES=n/a
PiperOrigin-RevId: 702770479
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Jan 3, 2025
1 parent 80559d2 commit 9449e89
Show file tree
Hide file tree
Showing 3 changed files with 207 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.common.util.concurrent;

import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;

import com.google.common.collect.ImmutableSet;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
Expand All @@ -24,26 +26,19 @@
import junit.framework.TestCase;
import junit.framework.TestSuite;
import org.jspecify.annotations.NullUnmarked;
import sun.misc.Unsafe;

/**
* Tests our AtomicHelper fallback strategies in AbstractFuture.
*
* <p>On different platforms AbstractFuture uses different strategies for its core synchronization
* primitives. The strategies are all implemented as subtypes of AtomicHelper and the strategy is
* selected in the static initializer of AbstractFuture. This is convenient and performant but
* introduces some testing difficulties. This test exercises the two fallback strategies in abstract
* future.
*
* <ul>
* <li>SafeAtomicHelper: uses AtomicReferenceFieldsUpdaters to implement synchronization
* <li>SynchronizedHelper: uses {@code synchronized} blocks for synchronization
* </ul>
* introduces some testing difficulties. This test exercises the fallback strategies.
*
* To force selection of our fallback strategies we load {@link AbstractFuture} (and all of {@code
* com.google.common.util.concurrent}) in degenerate class loaders which make certain platform
* classes unavailable. Then we construct a test suite so we can run the normal AbstractFutureTest
* test methods in these degenerate classloaders.
* <p>To force selection of our fallback strategies, we load {@link AbstractFuture} (and all of
* {@code com.google.common.util.concurrent}) in degenerate class loaders which make certain
* platform classes unavailable. Then we construct a test suite so we can run the normal
* AbstractFutureTest test methods in these degenerate classloaders.
*/

@NullUnmarked
Expand All @@ -56,18 +51,16 @@ public class AbstractFutureFallbackAtomicHelperTest extends TestCase {
* This classloader disallows {@link sun.misc.Unsafe}, which will prevent us from selecting our
* preferred strategy {@code UnsafeAtomicHelper}.
*/
@SuppressWarnings({"SunApi", "removal"}) // b/345822163
private static final ClassLoader NO_UNSAFE =
getClassLoader(ImmutableSet.of(Unsafe.class.getName()));
private static final ClassLoader NO_UNSAFE = getClassLoader(ImmutableSet.of("sun.misc.Unsafe"));

/**
* This classloader disallows {@link sun.misc.Unsafe} and {@link AtomicReferenceFieldUpdater},
* which will prevent us from selecting our {@code SafeAtomicHelper} strategy.
* which will prevent us from selecting our {@code AtomicReferenceFieldUpdaterAtomicHelper}
* strategy.
*/
@SuppressWarnings({"SunApi", "removal"}) // b/345822163
private static final ClassLoader NO_ATOMIC_REFERENCE_FIELD_UPDATER =
getClassLoader(
ImmutableSet.of(Unsafe.class.getName(), AtomicReferenceFieldUpdater.class.getName()));
ImmutableSet.of("sun.misc.Unsafe", AtomicReferenceFieldUpdater.class.getName()));

public static TestSuite suite() {
// we create a test suite containing a test for every AbstractFutureTest test method and we
Expand Down Expand Up @@ -144,4 +137,8 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
}
};
}

private static boolean isJava8() {
return JAVA_SPECIFICATION_VERSION.value().equals("1.8");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.common.util.concurrent;

import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;

import com.google.common.collect.ImmutableSet;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
Expand All @@ -24,26 +26,19 @@
import junit.framework.TestCase;
import junit.framework.TestSuite;
import org.jspecify.annotations.NullUnmarked;
import sun.misc.Unsafe;

/**
* Tests our AtomicHelper fallback strategies in AbstractFuture.
*
* <p>On different platforms AbstractFuture uses different strategies for its core synchronization
* primitives. The strategies are all implemented as subtypes of AtomicHelper and the strategy is
* selected in the static initializer of AbstractFuture. This is convenient and performant but
* introduces some testing difficulties. This test exercises the two fallback strategies in abstract
* future.
*
* <ul>
* <li>SafeAtomicHelper: uses AtomicReferenceFieldsUpdaters to implement synchronization
* <li>SynchronizedHelper: uses {@code synchronized} blocks for synchronization
* </ul>
* introduces some testing difficulties. This test exercises the fallback strategies.
*
* To force selection of our fallback strategies we load {@link AbstractFuture} (and all of {@code
* com.google.common.util.concurrent}) in degenerate class loaders which make certain platform
* classes unavailable. Then we construct a test suite so we can run the normal AbstractFutureTest
* test methods in these degenerate classloaders.
* <p>To force selection of our fallback strategies, we load {@link AbstractFuture} (and all of
* {@code com.google.common.util.concurrent}) in degenerate class loaders which make certain
* platform classes unavailable. Then we construct a test suite so we can run the normal
* AbstractFutureTest test methods in these degenerate classloaders.
*/

@NullUnmarked
Expand All @@ -53,21 +48,30 @@ public class AbstractFutureFallbackAtomicHelperTest extends TestCase {
// execution significantly)

/**
* This classloader disallows {@link sun.misc.Unsafe}, which will prevent us from selecting our
* preferred strategy {@code UnsafeAtomicHelper}.
* This classloader disallows {@link java.lang.invoke.VarHandle}, which will prevent us from
* selecting our preferred strategy {@code VarHandleAtomicHelper}.
*/
private static final ClassLoader NO_VAR_HANDLE =
getClassLoader(ImmutableSet.of("java.lang.invoke.VarHandle"));

/**
* This classloader disallows {@link java.lang.invoke.VarHandle} and {@link sun.misc.Unsafe},
* which will prevent us from selecting our {@code UnsafeAtomicHelper} strategy.
*/
@SuppressWarnings({"SunApi", "removal"}) // b/345822163
private static final ClassLoader NO_UNSAFE =
getClassLoader(ImmutableSet.of(Unsafe.class.getName()));
getClassLoader(ImmutableSet.of("java.lang.invoke.VarHandle", "sun.misc.Unsafe"));

/**
* This classloader disallows {@link sun.misc.Unsafe} and {@link AtomicReferenceFieldUpdater},
* which will prevent us from selecting our {@code SafeAtomicHelper} strategy.
* This classloader disallows {@link java.lang.invoke.VarHandle}, {@link sun.misc.Unsafe} and
* {@link AtomicReferenceFieldUpdater}, which will prevent us from selecting our {@code
* AtomicReferenceFieldUpdaterAtomicHelper} strategy.
*/
@SuppressWarnings({"SunApi", "removal"}) // b/345822163
private static final ClassLoader NO_ATOMIC_REFERENCE_FIELD_UPDATER =
getClassLoader(
ImmutableSet.of(Unsafe.class.getName(), AtomicReferenceFieldUpdater.class.getName()));
ImmutableSet.of(
"java.lang.invoke.VarHandle",
"sun.misc.Unsafe",
AtomicReferenceFieldUpdater.class.getName()));

public static TestSuite suite() {
// we create a test suite containing a test for every AbstractFutureTest test method and we
Expand All @@ -86,13 +90,25 @@ public static TestSuite suite() {
@Override
public void runTest() throws Exception {
// First ensure that our classloaders are initializing the correct helper versions
checkHelperVersion(getClass().getClassLoader(), "UnsafeAtomicHelper");
if (isJava8()) {
checkHelperVersion(getClass().getClassLoader(), "UnsafeAtomicHelper");
} else {
checkHelperVersion(getClass().getClassLoader(), "VarHandleAtomicHelper");
}
checkHelperVersion(NO_VAR_HANDLE, "UnsafeAtomicHelper");
checkHelperVersion(NO_UNSAFE, "AtomicReferenceFieldUpdaterAtomicHelper");
checkHelperVersion(NO_ATOMIC_REFERENCE_FIELD_UPDATER, "SynchronizedHelper");

// Run the corresponding AbstractFutureTest test method in a new classloader that disallows
// certain core jdk classes.
ClassLoader oldClassLoader = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(NO_UNSAFE);
try {
runTestMethod(NO_VAR_HANDLE);
} finally {
Thread.currentThread().setContextClassLoader(oldClassLoader);
}

Thread.currentThread().setContextClassLoader(NO_UNSAFE);
try {
runTestMethod(NO_UNSAFE);
Expand Down Expand Up @@ -144,4 +160,8 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
}
};
}

private static boolean isJava8() {
return JAVA_SPECIFICATION_VERSION.value().equals("1.8");
}
}
174 changes: 151 additions & 23 deletions guava/src/com/google/common/util/concurrent/AbstractFuture.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@
import com.google.common.util.concurrent.internal.InternalFutures;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.annotations.ForOverride;
import com.google.j2objc.annotations.J2ObjCIncompatible;
import com.google.j2objc.annotations.ReflectionSupport;
import com.google.j2objc.annotations.RetainedLocalRef;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.VarHandle;
import java.lang.reflect.Field;
import java.security.AccessController;
import java.security.PrivilegedActionException;
Expand Down Expand Up @@ -150,35 +153,60 @@ public final boolean cancel(boolean mayInterruptIfRunning) {

private static final AtomicHelper ATOMIC_HELPER;

/**
* Returns the result of calling {@link MethodHandles#lookup} from inside {@link AbstractFuture}.
* By virtue of being created there, it has access to the private fields of {@link
* AbstractFuture}, so that access is available to anyone who calls this method—specifically, to
* {@link VarHandleAtomicHelper}.
*
* <p>This "shouldn't" be necessary: {@link VarHandleAtomicHelper} and {@link AbstractFuture}
* "should" be nestmates, so a call to {@link MethodHandles#lookup} inside {@link
* VarHandleAtomicHelper} "should" have access to each other's private fields. However, our
* open-source build uses {@code -source 8 -target 8}, so the class files from that build can't
* express nestmates. Thus, when those class files are used from Java 9 or higher (i.e., high
* enough to trigger the {@link VarHandle} code path), such a lookup would fail with an {@link
* IllegalAccessException}.
*
* <p>Note that we do not have a similar problem with the fields in {@link Waiter} because those
* fields are not private. (We could solve the problem with {@link AbstractFuture} fields in the
* same way if we wanted.)
*/
private static MethodHandles.Lookup methodHandlesLookupFromWithinAbstractFuture() {
return MethodHandles.lookup();
}

static {
AtomicHelper helper;
Throwable thrownUnsafeFailure = null;
Throwable thrownAtomicReferenceFieldUpdaterFailure = null;

try {
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
helper = VarHandleAtomicHelperMaker.INSTANCE.tryMakeVarHandleAtomicHelper();
if (helper == null) {
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"));
} 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.
thrownAtomicReferenceFieldUpdaterFailure = atomicReferenceFieldUpdaterFailure;
helper = new SynchronizedHelper();
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
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"));
} 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.
thrownAtomicReferenceFieldUpdaterFailure = atomicReferenceFieldUpdaterFailure;
helper = new SynchronizedHelper();
}
}
}
ATOMIC_HELPER = helper;
Expand All @@ -200,6 +228,40 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
}
}

private enum VarHandleAtomicHelperMaker {
INSTANCE {
/**
* Implementation used by non-J2ObjC environments (aside, of course, from those that have
* supersource for the entirety of {@link AbstractFuture}).
*/
@Override
@J2ObjCIncompatible
@Nullable AtomicHelper tryMakeVarHandleAtomicHelper() {
try {
/*
* We first use reflection to check whether VarHandle exists. If we instead just tried to
* load our class directly (which would trigger non-reflective loading of VarHandle) from
* within a `try` block, then an error might be thrown even before we enter the `try`
* block: https://github.com/google/truth/issues/333#issuecomment-765652454
*
* Also, it's nice that this approach should let us catch *only* ClassNotFoundException
* instead of having to catch more broadly (potentially even including, say, a
* StackOverflowError).
*/
Class.forName("java.lang.invoke.VarHandle");
} catch (ClassNotFoundException beforeJava9) {
return null;
}
return new VarHandleAtomicHelper();
}
};

/** Implementation used by J2ObjC environments, overridden for other environments. */
@Nullable AtomicHelper tryMakeVarHandleAtomicHelper() {
return null;
}
}

/** Waiter links form a Treiber stack, in the {@link #waiters} field. */
private static final class Waiter {
static final Waiter TOMBSTONE = new Waiter(false /* ignored param */);
Expand Down Expand Up @@ -1339,6 +1401,72 @@ abstract boolean casListeners(
abstract boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object update);
}

/** {@link AtomicHelper} based on {@link VarHandle}. */
@J2ObjCIncompatible
// We use this class only after confirming that VarHandle is available at runtime.
@SuppressWarnings("Java8ApiChecker")
@IgnoreJRERequirement
private static final class VarHandleAtomicHelper extends AtomicHelper {
static final VarHandle waiterThreadUpdater;
static final VarHandle waiterNextUpdater;
static final VarHandle waitersUpdater;
static final VarHandle listenersUpdater;
static final VarHandle valueUpdater;

static {
MethodHandles.Lookup lookup = methodHandlesLookupFromWithinAbstractFuture();
try {
waiterThreadUpdater = lookup.findVarHandle(Waiter.class, "thread", Thread.class);
waiterNextUpdater = lookup.findVarHandle(Waiter.class, "next", Waiter.class);
waitersUpdater = lookup.findVarHandle(AbstractFuture.class, "waiters", Waiter.class);
listenersUpdater = lookup.findVarHandle(AbstractFuture.class, "listeners", Listener.class);
valueUpdater = lookup.findVarHandle(AbstractFuture.class, "value", Object.class);
} catch (ReflectiveOperationException e) {
// Those fields exist.
throw newLinkageError(e);
}
}

@Override
void putThread(Waiter waiter, Thread newValue) {
waiterThreadUpdater.setRelease(waiter, newValue);
}

@Override
void putNext(Waiter waiter, @Nullable Waiter newValue) {
waiterNextUpdater.setRelease(waiter, newValue);
}

@Override
boolean casWaiters(AbstractFuture<?> future, @Nullable Waiter expect, @Nullable Waiter update) {
return waitersUpdater.compareAndSet(future, expect, update);
}

@Override
boolean casListeners(AbstractFuture<?> future, @Nullable Listener expect, Listener update) {
return listenersUpdater.compareAndSet(future, expect, update);
}

@Override
Listener gasListeners(AbstractFuture<?> future, Listener update) {
return (Listener) listenersUpdater.getAndSet(future, update);
}

@Override
Waiter gasWaiters(AbstractFuture<?> future, Waiter update) {
return (Waiter) waitersUpdater.getAndSet(future, update);
}

@Override
boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object update) {
return valueUpdater.compareAndSet(future, expect, update);
}

private static LinkageError newLinkageError(Throwable cause) {
return new LinkageError(cause.toString(), cause);
}
}

/**
* {@link AtomicHelper} based on {@link sun.misc.Unsafe}.
*
Expand Down

0 comments on commit 9449e89

Please sign in to comment.