Skip to content

Commit

Permalink
Stop computing a fingerprint in FragmentClassSet.
Browse files Browse the repository at this point in the history
Now that the hash code is precomputed, the only time the fingerprint would be helpful is if there is a hash collision for unequal instances.

The fingerprint computation shows up as wasteful when enabling required config fragments.

PiperOrigin-RevId: 400012814
  • Loading branch information
justinhorvitz authored and copybara-github committed Sep 30, 2021
1 parent c2d6def commit a45c5ca
Showing 1 changed file with 4 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.util.ClassName;
import com.google.devtools.build.lib.util.Fingerprint;
import java.util.AbstractSet;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.Iterator;
Expand All @@ -35,7 +33,7 @@
/**
* A wrapper class for an {@code ImmutableSortedSet<Class<? extends Fragment>>}. Interning these
* objects allows us to do cheap reference equality checks when these sets are in frequently used
* keys. For good measure, we also compute a fingerprint.
* keys.
*/
@AutoCodec
@Immutable
Expand All @@ -55,9 +53,7 @@ public final class FragmentClassSet extends AbstractSet<Class<? extends Fragment
public static FragmentClassSet of(Collection<Class<? extends Fragment>> fragments) {
ImmutableSortedSet<Class<? extends Fragment>> sortedFragments =
ImmutableSortedSet.copyOf(LEXICAL_FRAGMENT_SORTER, fragments);
byte[] fingerprint = computeFingerprint(sortedFragments);
return interner.intern(
new FragmentClassSet(sortedFragments, fingerprint, Arrays.hashCode(fingerprint)));
return interner.intern(new FragmentClassSet(sortedFragments, sortedFragments.hashCode()));
}

public static FragmentClassSet union(
Expand All @@ -69,23 +65,11 @@ public static FragmentClassSet union(
.build());
}

private static byte[] computeFingerprint(
ImmutableSortedSet<Class<? extends Fragment>> fragments) {
Fingerprint fingerprint = new Fingerprint();
for (Class<?> fragment : fragments) {
fingerprint.addString(fragment.getName());
}
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) {
private FragmentClassSet(ImmutableSortedSet<Class<? extends Fragment>> fragments, int hashCode) {
this.fragments = fragments;
this.fingerprint = fingerprint;
this.hashCode = hashCode;
}

Expand Down Expand Up @@ -113,7 +97,6 @@ public Iterator<Class<? extends Fragment>> iterator() {
}

@Override
@SuppressWarnings("ReferenceEquality") // Fast-path check of the underlying fragments set.
public boolean equals(Object other) {
if (this == other) {
return true;
Expand All @@ -122,7 +105,7 @@ public boolean equals(Object other) {
return false;
}
FragmentClassSet that = (FragmentClassSet) other;
return fragments == that.fragments || Arrays.equals(fingerprint, that.fingerprint);
return hashCode == that.hashCode && fragments.equals(that.fragments);
}

@Override
Expand Down

0 comments on commit a45c5ca

Please sign in to comment.