Precompute the hash code and fingerprint in `FragmentClassSet`.
Like since it is always interned, there's no reason to lazily compute the hash code - it will be called by the interner.
@@ -22,7 +22,6 @@ import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.Fingerprint;
import java.util.Arrays;
-import javax.annotation.Nullable;
/**
* A wrapper class for an {@code ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>>}
@@ -30,72 +29,56 @@ import javax.annotation.Nullable;
* are in frequently used keys. For good measure, we also compute a fingerprint.
*/
@AutoCodec
-public class FragmentClassSet {
- private final ImmutableSortedSet<Class<? extends Fragment>> fragments;
-
- // Lazily initialized.
- @Nullable private volatile byte[] fingerprint;
- private volatile int hashCode;
-
- private FragmentClassSet(ImmutableSortedSet<Class<? extends Fragment>> fragments) {
- this.fragments = fragments;
- }
+public final class FragmentClassSet {
private static final Interner<FragmentClassSet> interner = BlazeInterners.newWeakInterner();
@AutoCodec.Instantiator
public static FragmentClassSet of(ImmutableSortedSet<Class<? extends Fragment>> fragments) {
- return interner.intern(new FragmentClassSet(fragments));
- }
-
- public ImmutableSortedSet<Class<? extends Fragment>> fragmentClasses() {
- return fragments;
+ byte[] fingerprint = computeFingerprint(fragments);
+ return interner.intern(
+ new FragmentClassSet(fragments, fingerprint, Arrays.hashCode(fingerprint)));
}
- /**
- * Lazily initialize {@link #fingerprint} and {@link #hashCode}. Keeps computation off critical
- * path of build, while still avoiding expensive computation for equality and hash code each time.
- *
- * <p>We check for nullity of {@link #fingerprint} to see if this method has already been called.
- * Using {@link #hashCode} after this method is called is safe because it is set here before
- * {@link #fingerprint} is set, so if {@link #fingerprint} is non-null then {@link #hashCode} is
- * definitely set.
- */
- private void maybeInitializeFingerprintAndHashCode() {
- if (fingerprint != null) {
- return;
- }
- synchronized (this) {
- if (fingerprint != null) {
- return;
- }
+ private static byte[] computeFingerprint(
+ ImmutableSortedSet<Class<? extends Fragment>> fragments) {
Fingerprint fingerprint = new Fingerprint();
- for (Class<? extends Fragment> fragment : fragments) {
+ for (Class<?> fragment : fragments) {
fingerprint.addString(fragment.getName());
}
- byte[] computedFingerprint = fingerprint.digestAndReset();
- hashCode = Arrays.hashCode(computedFingerprint);
- this.fingerprint = computedFingerprint;
+ return fingerprint.digestAndReset();
+ }
+
+ private final ImmutableSortedSet<Class<? extends Fragment>> fragments;
+ private final byte[] fingerprint;
+ private final int hashCode;
+
+ private FragmentClassSet(
+ ImmutableSortedSet<Class<? extends Fragment>> fragments, byte[] fingerprint, int hashCode) {
+ this.fragments = fragments;
+ this.fingerprint = fingerprint;
+ this.hashCode = hashCode;
}
+
+ public ImmutableSortedSet<Class<? extends Fragment>> fragmentClasses() {
+ return fragments;
}
@Override
+ @SuppressWarnings("ReferenceEquality") // Fast-path check of the underlying fragments set.
public boolean equals(Object other) {
if (this == other) {
return true;
- } else if (!(other instanceof FragmentClassSet)) {
+ }
+ if (!(other instanceof FragmentClassSet)) {
return false;
- } else {
- maybeInitializeFingerprintAndHashCode();
- FragmentClassSet that = (FragmentClassSet) other;
- that.maybeInitializeFingerprintAndHashCode();
- return Arrays.equals(this.fingerprint, that.fingerprint);
}
+ FragmentClassSet that = (FragmentClassSet) other;
+ return fragments == that.fragments || Arrays.equals(fingerprint, that.fingerprint);
}
@Override
public int hashCode() {
- maybeInitializeFingerprintAndHashCode();
return hashCode;
}
