Skip to content

Commit 1a300f6

Browse files
cpovirkGoogle Java Core Libraries
authored and
Google Java Core Libraries
committed
Make AbstractFuture use VarHandle when available.
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: 711733182
1 parent 80559d2 commit 1a300f6

File tree

3 files changed

+207
-62
lines changed

3 files changed

+207
-62
lines changed

android/guava-tests/test/com/google/common/util/concurrent/AbstractFutureFallbackAtomicHelperTest.java

+15-18
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
package com.google.common.util.concurrent;
1616

17+
import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
18+
1719
import com.google.common.collect.ImmutableSet;
1820
import java.lang.reflect.Field;
1921
import java.lang.reflect.Method;
@@ -24,26 +26,19 @@
2426
import junit.framework.TestCase;
2527
import junit.framework.TestSuite;
2628
import org.jspecify.annotations.NullUnmarked;
27-
import sun.misc.Unsafe;
2829

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

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

6356
/**
6457
* This classloader disallows {@link sun.misc.Unsafe} and {@link AtomicReferenceFieldUpdater},
65-
* which will prevent us from selecting our {@code SafeAtomicHelper} strategy.
58+
* which will prevent us from selecting our {@code AtomicReferenceFieldUpdaterAtomicHelper}
59+
* strategy.
6660
*/
67-
@SuppressWarnings({"SunApi", "removal"}) // b/345822163
6861
private static final ClassLoader NO_ATOMIC_REFERENCE_FIELD_UPDATER =
6962
getClassLoader(
70-
ImmutableSet.of(Unsafe.class.getName(), AtomicReferenceFieldUpdater.class.getName()));
63+
ImmutableSet.of("sun.misc.Unsafe", AtomicReferenceFieldUpdater.class.getName()));
7164

7265
public static TestSuite suite() {
7366
// we create a test suite containing a test for every AbstractFutureTest test method and we
@@ -144,4 +137,8 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
144137
}
145138
};
146139
}
140+
141+
private static boolean isJava8() {
142+
return JAVA_SPECIFICATION_VERSION.value().equals("1.8");
143+
}
147144
}

guava-tests/test/com/google/common/util/concurrent/AbstractFutureFallbackAtomicHelperTest.java

+41-21
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
package com.google.common.util.concurrent;
1616

17+
import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
18+
1719
import com.google.common.collect.ImmutableSet;
1820
import java.lang.reflect.Field;
1921
import java.lang.reflect.Method;
@@ -24,26 +26,19 @@
2426
import junit.framework.TestCase;
2527
import junit.framework.TestSuite;
2628
import org.jspecify.annotations.NullUnmarked;
27-
import sun.misc.Unsafe;
2829

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

4944
@NullUnmarked
@@ -53,21 +48,30 @@ public class AbstractFutureFallbackAtomicHelperTest extends TestCase {
5348
// execution significantly)
5449

5550
/**
56-
* This classloader disallows {@link sun.misc.Unsafe}, which will prevent us from selecting our
57-
* preferred strategy {@code UnsafeAtomicHelper}.
51+
* This classloader disallows {@link java.lang.invoke.VarHandle}, which will prevent us from
52+
* selecting our preferred strategy {@code VarHandleAtomicHelper}.
53+
*/
54+
private static final ClassLoader NO_VAR_HANDLE =
55+
getClassLoader(ImmutableSet.of("java.lang.invoke.VarHandle"));
56+
57+
/**
58+
* This classloader disallows {@link java.lang.invoke.VarHandle} and {@link sun.misc.Unsafe},
59+
* which will prevent us from selecting our {@code UnsafeAtomicHelper} strategy.
5860
*/
59-
@SuppressWarnings({"SunApi", "removal"}) // b/345822163
6061
private static final ClassLoader NO_UNSAFE =
61-
getClassLoader(ImmutableSet.of(Unsafe.class.getName()));
62+
getClassLoader(ImmutableSet.of("java.lang.invoke.VarHandle", "sun.misc.Unsafe"));
6263

6364
/**
64-
* This classloader disallows {@link sun.misc.Unsafe} and {@link AtomicReferenceFieldUpdater},
65-
* which will prevent us from selecting our {@code SafeAtomicHelper} strategy.
65+
* This classloader disallows {@link java.lang.invoke.VarHandle}, {@link sun.misc.Unsafe} and
66+
* {@link AtomicReferenceFieldUpdater}, which will prevent us from selecting our {@code
67+
* AtomicReferenceFieldUpdaterAtomicHelper} strategy.
6668
*/
67-
@SuppressWarnings({"SunApi", "removal"}) // b/345822163
6869
private static final ClassLoader NO_ATOMIC_REFERENCE_FIELD_UPDATER =
6970
getClassLoader(
70-
ImmutableSet.of(Unsafe.class.getName(), AtomicReferenceFieldUpdater.class.getName()));
71+
ImmutableSet.of(
72+
"java.lang.invoke.VarHandle",
73+
"sun.misc.Unsafe",
74+
AtomicReferenceFieldUpdater.class.getName()));
7175

7276
public static TestSuite suite() {
7377
// we create a test suite containing a test for every AbstractFutureTest test method and we
@@ -86,13 +90,25 @@ public static TestSuite suite() {
8690
@Override
8791
public void runTest() throws Exception {
8892
// First ensure that our classloaders are initializing the correct helper versions
89-
checkHelperVersion(getClass().getClassLoader(), "UnsafeAtomicHelper");
93+
if (isJava8()) {
94+
checkHelperVersion(getClass().getClassLoader(), "UnsafeAtomicHelper");
95+
} else {
96+
checkHelperVersion(getClass().getClassLoader(), "VarHandleAtomicHelper");
97+
}
98+
checkHelperVersion(NO_VAR_HANDLE, "UnsafeAtomicHelper");
9099
checkHelperVersion(NO_UNSAFE, "AtomicReferenceFieldUpdaterAtomicHelper");
91100
checkHelperVersion(NO_ATOMIC_REFERENCE_FIELD_UPDATER, "SynchronizedHelper");
92101

93102
// Run the corresponding AbstractFutureTest test method in a new classloader that disallows
94103
// certain core jdk classes.
95104
ClassLoader oldClassLoader = Thread.currentThread().getContextClassLoader();
105+
Thread.currentThread().setContextClassLoader(NO_UNSAFE);
106+
try {
107+
runTestMethod(NO_VAR_HANDLE);
108+
} finally {
109+
Thread.currentThread().setContextClassLoader(oldClassLoader);
110+
}
111+
96112
Thread.currentThread().setContextClassLoader(NO_UNSAFE);
97113
try {
98114
runTestMethod(NO_UNSAFE);
@@ -144,4 +160,8 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
144160
}
145161
};
146162
}
163+
164+
private static boolean isJava8() {
165+
return JAVA_SPECIFICATION_VERSION.value().equals("1.8");
166+
}
147167
}

guava/src/com/google/common/util/concurrent/AbstractFuture.java

+151-23
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,11 @@
2929
import com.google.common.util.concurrent.internal.InternalFutures;
3030
import com.google.errorprone.annotations.CanIgnoreReturnValue;
3131
import com.google.errorprone.annotations.ForOverride;
32+
import com.google.j2objc.annotations.J2ObjCIncompatible;
3233
import com.google.j2objc.annotations.ReflectionSupport;
3334
import com.google.j2objc.annotations.RetainedLocalRef;
35+
import java.lang.invoke.MethodHandles;
36+
import java.lang.invoke.VarHandle;
3437
import java.lang.reflect.Field;
3538
import java.security.AccessController;
3639
import java.security.PrivilegedActionException;
@@ -150,35 +153,60 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
150153

151154
private static final AtomicHelper ATOMIC_HELPER;
152155

156+
/**
157+
* Returns the result of calling {@link MethodHandles#lookup} from inside {@link AbstractFuture}.
158+
* By virtue of being created there, it has access to the private fields of {@link
159+
* AbstractFuture}, so that access is available to anyone who calls this method—specifically, to
160+
* {@link VarHandleAtomicHelper}.
161+
*
162+
* <p>This "shouldn't" be necessary: {@link VarHandleAtomicHelper} and {@link AbstractFuture}
163+
* "should" be nestmates, so a call to {@link MethodHandles#lookup} inside {@link
164+
* VarHandleAtomicHelper} "should" have access to each other's private fields. However, our
165+
* open-source build uses {@code -source 8 -target 8}, so the class files from that build can't
166+
* express nestmates. Thus, when those class files are used from Java 9 or higher (i.e., high
167+
* enough to trigger the {@link VarHandle} code path), such a lookup would fail with an {@link
168+
* IllegalAccessException}.
169+
*
170+
* <p>Note that we do not have a similar problem with the fields in {@link Waiter} because those
171+
* fields are not private. (We could solve the problem with {@link AbstractFuture} fields in the
172+
* same way if we wanted.)
173+
*/
174+
private static MethodHandles.Lookup methodHandlesLookupFromWithinAbstractFuture() {
175+
return MethodHandles.lookup();
176+
}
177+
153178
static {
154179
AtomicHelper helper;
155180
Throwable thrownUnsafeFailure = null;
156181
Throwable thrownAtomicReferenceFieldUpdaterFailure = null;
157182

158-
try {
159-
helper = new UnsafeAtomicHelper();
160-
} catch (Exception | Error unsafeFailure) { // sneaky checked exception
161-
thrownUnsafeFailure = unsafeFailure;
162-
// catch absolutely everything and fall through to our
163-
// 'AtomicReferenceFieldUpdaterAtomicHelper' The access control checks that ARFU does means
164-
// the caller class has to be AbstractFuture instead of
165-
// AtomicReferenceFieldUpdaterAtomicHelper, so we annoyingly define these here
183+
helper = VarHandleAtomicHelperMaker.INSTANCE.tryMakeVarHandleAtomicHelper();
184+
if (helper == null) {
166185
try {
167-
helper =
168-
new AtomicReferenceFieldUpdaterAtomicHelper(
169-
newUpdater(Waiter.class, Thread.class, "thread"),
170-
newUpdater(Waiter.class, Waiter.class, "next"),
171-
newUpdater(AbstractFuture.class, Waiter.class, "waiters"),
172-
newUpdater(AbstractFuture.class, Listener.class, "listeners"),
173-
newUpdater(AbstractFuture.class, Object.class, "value"));
174-
} catch (Exception // sneaky checked exception
175-
| Error atomicReferenceFieldUpdaterFailure) {
176-
// Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
177-
// getDeclaredField to throw a NoSuchFieldException when the field is definitely there.
178-
// For these users fallback to a suboptimal implementation, based on synchronized. This will
179-
// be a definite performance hit to those users.
180-
thrownAtomicReferenceFieldUpdaterFailure = atomicReferenceFieldUpdaterFailure;
181-
helper = new SynchronizedHelper();
186+
helper = new UnsafeAtomicHelper();
187+
} catch (Exception | Error unsafeFailure) { // sneaky checked exception
188+
thrownUnsafeFailure = unsafeFailure;
189+
// catch absolutely everything and fall through to our
190+
// 'AtomicReferenceFieldUpdaterAtomicHelper' The access control checks that ARFU does means
191+
// the caller class has to be AbstractFuture instead of
192+
// AtomicReferenceFieldUpdaterAtomicHelper, so we annoyingly define these here
193+
try {
194+
helper =
195+
new AtomicReferenceFieldUpdaterAtomicHelper(
196+
newUpdater(Waiter.class, Thread.class, "thread"),
197+
newUpdater(Waiter.class, Waiter.class, "next"),
198+
newUpdater(AbstractFuture.class, Waiter.class, "waiters"),
199+
newUpdater(AbstractFuture.class, Listener.class, "listeners"),
200+
newUpdater(AbstractFuture.class, Object.class, "value"));
201+
} catch (Exception // sneaky checked exception
202+
| Error atomicReferenceFieldUpdaterFailure) {
203+
// Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
204+
// getDeclaredField to throw a NoSuchFieldException when the field is definitely there.
205+
// For these users fallback to a suboptimal implementation, based on synchronized. This
206+
// will be a definite performance hit to those users.
207+
thrownAtomicReferenceFieldUpdaterFailure = atomicReferenceFieldUpdaterFailure;
208+
helper = new SynchronizedHelper();
209+
}
182210
}
183211
}
184212
ATOMIC_HELPER = helper;
@@ -200,6 +228,40 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
200228
}
201229
}
202230

231+
private enum VarHandleAtomicHelperMaker {
232+
INSTANCE {
233+
/**
234+
* Implementation used by non-J2ObjC environments (aside, of course, from those that have
235+
* supersource for the entirety of {@link AbstractFuture}).
236+
*/
237+
@Override
238+
@J2ObjCIncompatible
239+
@Nullable AtomicHelper tryMakeVarHandleAtomicHelper() {
240+
try {
241+
/*
242+
* We first use reflection to check whether VarHandle exists. If we instead just tried to
243+
* load our class directly (which would trigger non-reflective loading of VarHandle) from
244+
* within a `try` block, then an error might be thrown even before we enter the `try`
245+
* block: https://github.com/google/truth/issues/333#issuecomment-765652454
246+
*
247+
* Also, it's nice that this approach should let us catch *only* ClassNotFoundException
248+
* instead of having to catch more broadly (potentially even including, say, a
249+
* StackOverflowError).
250+
*/
251+
Class.forName("java.lang.invoke.VarHandle");
252+
} catch (ClassNotFoundException beforeJava9) {
253+
return null;
254+
}
255+
return new VarHandleAtomicHelper();
256+
}
257+
};
258+
259+
/** Implementation used by J2ObjC environments, overridden for other environments. */
260+
@Nullable AtomicHelper tryMakeVarHandleAtomicHelper() {
261+
return null;
262+
}
263+
}
264+
203265
/** Waiter links form a Treiber stack, in the {@link #waiters} field. */
204266
private static final class Waiter {
205267
static final Waiter TOMBSTONE = new Waiter(false /* ignored param */);
@@ -1339,6 +1401,72 @@ abstract boolean casListeners(
13391401
abstract boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object update);
13401402
}
13411403

1404+
/** {@link AtomicHelper} based on {@link VarHandle}. */
1405+
@J2ObjCIncompatible
1406+
// We use this class only after confirming that VarHandle is available at runtime.
1407+
@SuppressWarnings("Java8ApiChecker")
1408+
@IgnoreJRERequirement
1409+
private static final class VarHandleAtomicHelper extends AtomicHelper {
1410+
static final VarHandle waiterThreadUpdater;
1411+
static final VarHandle waiterNextUpdater;
1412+
static final VarHandle waitersUpdater;
1413+
static final VarHandle listenersUpdater;
1414+
static final VarHandle valueUpdater;
1415+
1416+
static {
1417+
MethodHandles.Lookup lookup = methodHandlesLookupFromWithinAbstractFuture();
1418+
try {
1419+
waiterThreadUpdater = lookup.findVarHandle(Waiter.class, "thread", Thread.class);
1420+
waiterNextUpdater = lookup.findVarHandle(Waiter.class, "next", Waiter.class);
1421+
waitersUpdater = lookup.findVarHandle(AbstractFuture.class, "waiters", Waiter.class);
1422+
listenersUpdater = lookup.findVarHandle(AbstractFuture.class, "listeners", Listener.class);
1423+
valueUpdater = lookup.findVarHandle(AbstractFuture.class, "value", Object.class);
1424+
} catch (ReflectiveOperationException e) {
1425+
// Those fields exist.
1426+
throw newLinkageError(e);
1427+
}
1428+
}
1429+
1430+
@Override
1431+
void putThread(Waiter waiter, Thread newValue) {
1432+
waiterThreadUpdater.setRelease(waiter, newValue);
1433+
}
1434+
1435+
@Override
1436+
void putNext(Waiter waiter, @Nullable Waiter newValue) {
1437+
waiterNextUpdater.setRelease(waiter, newValue);
1438+
}
1439+
1440+
@Override
1441+
boolean casWaiters(AbstractFuture<?> future, @Nullable Waiter expect, @Nullable Waiter update) {
1442+
return waitersUpdater.compareAndSet(future, expect, update);
1443+
}
1444+
1445+
@Override
1446+
boolean casListeners(AbstractFuture<?> future, @Nullable Listener expect, Listener update) {
1447+
return listenersUpdater.compareAndSet(future, expect, update);
1448+
}
1449+
1450+
@Override
1451+
Listener gasListeners(AbstractFuture<?> future, Listener update) {
1452+
return (Listener) listenersUpdater.getAndSet(future, update);
1453+
}
1454+
1455+
@Override
1456+
Waiter gasWaiters(AbstractFuture<?> future, Waiter update) {
1457+
return (Waiter) waitersUpdater.getAndSet(future, update);
1458+
}
1459+
1460+
@Override
1461+
boolean casValue(AbstractFuture<?> future, @Nullable Object expect, Object update) {
1462+
return valueUpdater.compareAndSet(future, expect, update);
1463+
}
1464+
1465+
private static LinkageError newLinkageError(Throwable cause) {
1466+
return new LinkageError(cause.toString(), cause);
1467+
}
1468+
}
1469+
13421470
/**
13431471
* {@link AtomicHelper} based on {@link sun.misc.Unsafe}.
13441472
*

0 commit comments

Comments
 (0)