From 9d3b3bfa917902485da1e491910324ffad36e1dd Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 27 Feb 2023 13:20:23 -0800 Subject: [PATCH] Refactor the generic `GroupedList` into `GroupedDeps`, specifically designed to hold skyframe dependency groups. `GroupedList` is only ever used as `GroupedList` except for tests. Refactor it so that it can be better tailored to this use case instead of pretending to be a general-purpose data structure. PiperOrigin-RevId: 512713368 Change-Id: If0760c7294bcf53afd79c01043bf91e4d2387aa9 --- .../ProgressEventSuppressingEnvironment.java | 4 +- .../StateInformingSkyFunctionEnvironment.java | 4 +- .../skyframe/TransitiveTraversalFunction.java | 6 +- .../com/google/devtools/build/lib/util/BUILD | 3 - .../AbstractInMemoryMemoizingEvaluator.java | 3 +- .../com/google/devtools/build/skyframe/BUILD | 5 + .../build/skyframe/DelegatingNodeEntry.java | 3 +- .../build/skyframe/DirtyBuildingState.java | 14 +- .../GroupedDeps.java} | 140 +++++----- .../build/skyframe/InMemoryNodeEntry.java | 42 ++- .../devtools/build/skyframe/NodeEntry.java | 5 +- .../build/skyframe/QueryableGraph.java | 6 +- .../RecordingSkyFunctionEnvironment.java | 3 +- .../build/skyframe/SimpleCycleDetector.java | 7 +- .../devtools/build/skyframe/SkyFunction.java | 3 +- .../skyframe/SkyFunctionEnvironment.java | 17 +- .../TransitiveTraversalFunctionTest.java | 18 +- .../build/lib/util/GroupedListTest.java | 246 ----------------- .../build/skyframe/GroupedDepsTest.java | 257 ++++++++++++++++++ .../build/skyframe/InMemoryNodeEntryTest.java | 9 +- .../build/skyframe/NotifyingHelper.java | 3 +- 21 files changed, 397 insertions(+), 401 deletions(-) rename src/main/java/com/google/devtools/build/{lib/util/GroupedList.java => skyframe/GroupedDeps.java} (78%) delete mode 100644 src/test/java/com/google/devtools/build/lib/util/GroupedListTest.java create mode 100644 src/test/java/com/google/devtools/build/skyframe/GroupedDepsTest.java diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ProgressEventSuppressingEnvironment.java b/src/main/java/com/google/devtools/build/lib/skyframe/ProgressEventSuppressingEnvironment.java index c0c1195cc7ff9b..7a1339d43c01eb 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ProgressEventSuppressingEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ProgressEventSuppressingEnvironment.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.util.concurrent.ListenableFuture; -import com.google.devtools.build.lib.util.GroupedList; +import com.google.devtools.build.skyframe.GroupedDeps; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; @@ -104,7 +104,7 @@ public SkyframeLookupResult getValuesAndExceptions(Iterable de @Override @Nullable - public GroupedList getTemporaryDirectDeps() { + public GroupedDeps getTemporaryDirectDeps() { return delegate.getTemporaryDirectDeps(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StateInformingSkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/lib/skyframe/StateInformingSkyFunctionEnvironment.java index 82709e96a75a42..38d77e486aca74 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/StateInformingSkyFunctionEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/StateInformingSkyFunctionEnvironment.java @@ -15,7 +15,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.events.ExtendedEventHandler; -import com.google.devtools.build.lib.util.GroupedList; +import com.google.devtools.build.skyframe.GroupedDeps; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; @@ -137,7 +137,7 @@ public boolean inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors() { @Nullable @Override - public GroupedList getTemporaryDirectDeps() { + public GroupedDeps getTemporaryDirectDeps() { return delegate.getTemporaryDirectDeps(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java index 0205e2f780ffc6..6efe2414e2d5b3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java @@ -21,7 +21,7 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.skyframe.TargetLoadingUtil.TargetAndErrorIfAny; -import com.google.devtools.build.lib.util.GroupedList; +import com.google.devtools.build.skyframe.GroupedDeps; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; @@ -140,7 +140,7 @@ Iterable getStrictLabelAspectDepKeys( @Nullable private static Collection getDepsAfterLastPackageDep( SkyFunction.Environment env, int offset) { - GroupedList temporaryDirectDeps = env.getTemporaryDirectDeps(); + GroupedDeps temporaryDirectDeps = env.getTemporaryDirectDeps(); if (temporaryDirectDeps == null) { return null; } @@ -152,7 +152,7 @@ private static Collection getDepsAfterLastPackageDep( return temporaryDirectDeps.get(lastPackageDepIndex + offset); } - private static int getLastPackageValueIndex(GroupedList directDeps) { + private static int getLastPackageValueIndex(GroupedDeps directDeps) { int directDepsNumGroups = directDeps.listSize(); for (int i = directDepsNumGroups - 1; i >= 0; i--) { List depGroup = directDeps.get(i); diff --git a/src/main/java/com/google/devtools/build/lib/util/BUILD b/src/main/java/com/google/devtools/build/lib/util/BUILD index 02c34eb6014739..6e379bc1ca150e 100644 --- a/src/main/java/com/google/devtools/build/lib/util/BUILD +++ b/src/main/java/com/google/devtools/build/lib/util/BUILD @@ -165,7 +165,6 @@ java_library( "Either.java", "FileHandlerQuerier.java", "Fingerprint.java", - "GroupedList.java", "JavaSleeper.java", "LogHandlerQuerier.java", "LoggingUtil.java", @@ -195,10 +194,8 @@ java_library( ":shell_escaper", ":string", "//src/main/java/com/google/devtools/build/lib/bugreport", - "//src/main/java/com/google/devtools/build/lib/collect/compacthashset", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", - "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java index 1a1959cfb0812e..50cefa4b2102e4 100644 --- a/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractInMemoryMemoizingEvaluator.java @@ -14,7 +14,6 @@ package com.google.devtools.build.skyframe; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.QueryableGraph.Reason; import java.io.PrintStream; import java.util.Collection; @@ -113,7 +112,7 @@ public final void dumpDeps(PrintStream out, Predicate filter) } out.println(canonicalizedKey); if (entry.keepsEdges()) { - GroupedList deps = GroupedList.create(entry.getCompressedDirectDepsForDoneEntry()); + GroupedDeps deps = GroupedDeps.create(entry.getCompressedDirectDepsForDoneEntry()); for (int i = 0; i < deps.listSize(); i++) { out.format(" Group %d:\n", i + 1); for (SkyKey dep : deps.get(i)) { diff --git a/src/main/java/com/google/devtools/build/skyframe/BUILD b/src/main/java/com/google/devtools/build/skyframe/BUILD index fbcf6d5adc48b9..6b6ed196f15aa8 100644 --- a/src/main/java/com/google/devtools/build/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/skyframe/BUILD @@ -11,6 +11,7 @@ package( SKYFRAME_OBJECT_SRCS = [ "AbstractSkyKey.java", "FunctionHermeticity.java", + "GroupedDeps.java", "NodeVersion.java", "SkyFunctionName.java", "SkyKey.java", @@ -24,8 +25,12 @@ java_library( srcs = SKYFRAME_OBJECT_SRCS, visibility = ["//visibility:public"], deps = [ + "//src/main/java/com/google/devtools/build/lib/collect/compacthashset", "//src/main/java/com/google/devtools/build/lib/concurrent", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", "//third_party:auto_value", + "//third_party:checker_framework_annotations", "//third_party:guava", ], ) diff --git a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java index b2741e6345838a..1a900e45d25795 100644 --- a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java @@ -15,7 +15,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.util.GroupedList; import java.util.List; import java.util.Set; import javax.annotation.Nullable; @@ -125,7 +124,7 @@ public void markRebuilding() { } @Override - public GroupedList getTemporaryDirectDeps() { + public GroupedDeps getTemporaryDirectDeps() { return getDelegate().getTemporaryDirectDeps(); } diff --git a/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java b/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java index ee53e2f2a53cf1..2b67682b232969 100644 --- a/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java +++ b/src/main/java/com/google/devtools/build/skyframe/DirtyBuildingState.java @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.unsafe.UnsafeProvider; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.NodeEntry.DirtyState; import com.google.devtools.build.skyframe.NodeEntry.DirtyType; import javax.annotation.Nullable; @@ -39,7 +38,7 @@ public abstract class DirtyBuildingState implements PriorityTracker { static DirtyBuildingState create( DirtyType dirtyType, - GroupedList lastBuildDirectDeps, + GroupedDeps lastBuildDirectDeps, SkyValue lastBuildValue, boolean hasLowFanout) { return new FullDirtyBuildingState(dirtyType, lastBuildDirectDeps, lastBuildValue, hasLowFanout); @@ -120,7 +119,7 @@ static DirtyBuildingState createNew(boolean hasLowFanout) { *

Public only for the use of alternative graph implementations. */ @Nullable - public abstract GroupedList getLastBuildDirectDeps() throws InterruptedException; + public abstract GroupedDeps getLastBuildDirectDeps() throws InterruptedException; /** * The number of groups of the dependencies requested last time when the node was built. @@ -271,8 +270,7 @@ && getLastBuildValue() != null * Returns true if the deps requested during this evaluation ({@code directDeps}) are exactly * those requested the last time this node was built, in the same order. */ - final boolean depsUnchangedFromLastBuild(GroupedList directDeps) - throws InterruptedException { + final boolean depsUnchangedFromLastBuild(GroupedDeps directDeps) throws InterruptedException { checkFinishedBuildingWhenAboutToSetValue(); return getLastBuildDirectDeps().equals(directDeps); } @@ -438,12 +436,12 @@ public String toString() { } private static class FullDirtyBuildingState extends DirtyBuildingState { - private final GroupedList lastBuildDirectDeps; + private final GroupedDeps lastBuildDirectDeps; private final SkyValue lastBuildValue; private FullDirtyBuildingState( DirtyType dirtyType, - GroupedList lastBuildDirectDeps, + GroupedDeps lastBuildDirectDeps, SkyValue lastBuildValue, boolean hasLowFanout) { super(dirtyType, hasLowFanout); @@ -466,7 +464,7 @@ public SkyValue getLastBuildValue() { } @Override - public GroupedList getLastBuildDirectDeps() throws InterruptedException { + public GroupedDeps getLastBuildDirectDeps() throws InterruptedException { return lastBuildDirectDeps; } diff --git a/src/main/java/com/google/devtools/build/lib/util/GroupedList.java b/src/main/java/com/google/devtools/build/skyframe/GroupedDeps.java similarity index 78% rename from src/main/java/com/google/devtools/build/lib/util/GroupedList.java rename to src/main/java/com/google/devtools/build/skyframe/GroupedDeps.java index ff15548cdf800b..2661f4f388fd42 100644 --- a/src/main/java/com/google/devtools/build/lib/util/GroupedList.java +++ b/src/main/java/com/google/devtools/build/skyframe/GroupedDeps.java @@ -11,7 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.util; +package com.google.devtools.build.skyframe; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; @@ -41,20 +41,19 @@ import org.checkerframework.framework.qual.SubtypeOf; /** - * Encapsulates a list of groups. + * Encapsulates Skyframe dependencies, preserving the groups in which they were requested. * - *

Despite the "list" name, it is an error for the same element to appear multiple times in the - * list. Users are responsible for not trying to add the same element to a GroupedList twice. + *

This class itself does no duplicate checking, although it is expected that a {@code + * GroupedDeps} instance contains no duplicates - Skyframe is responsible for only adding keys which + * are not already present. * *

Groups are implemented as lists to minimize memory use. However, {@link #equals} is defined to * treat groups as unordered. - * - *

The generic type {@code T} must not be a {@link List}. */ -public class GroupedList implements Iterable> { +public class GroupedDeps implements Iterable> { /** - * Indicates that the annotated element is a compressed {@link GroupedList}, so that it can be + * Indicates that the annotated element is a compressed {@link GroupedDeps}, so that it can be * safely passed to {@link #create} and friends. */ @SubtypeOf(DefaultObject.class) @@ -68,20 +67,20 @@ public class GroupedList implements Iterable> { @Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) private @interface DefaultObject {} - // Total number of items in the list. At least elements.size(), but might be larger if there are - // any nested lists. + // Total number of deps. At least elements.size(), but might be larger if there are any nested + // lists. private int size = 0; - // Items in this GroupedList. Each element is either of type T or List. + // Dep groups. Each element is either a SkyKey (a singleton group) or ImmutableList. private final ArrayList elements; private final CollectionView collectionView = new CollectionView(); - public GroupedList() { + public GroupedDeps() { // We optimize for small lists. this.elements = new ArrayList<>(1); } - private GroupedList(int size, Object[] elements) { + private GroupedDeps(int size, Object[] elements) { this.size = size; this.elements = Lists.newArrayList(elements); } @@ -90,7 +89,7 @@ private GroupedList(int size, Object[] elements) { * Increases the capacity of the backing list to accommodate the given number of additional * groups. */ - public void ensureCapacityForAdditionalGroups(int additionalGroups) { + void ensureCapacityForAdditionalGroups(int additionalGroups) { elements.ensureCapacity(elements.size() + additionalGroups); } @@ -99,9 +98,8 @@ public void ensureCapacityForAdditionalGroups(int additionalGroups) { * *

The caller must ensure that the given element is not already present. */ - public void appendSingleton(T t) { - checkArgument(!(t instanceof List), "Cannot created GroupedList of lists: %s", t); - elements.add(t); + public void appendSingleton(SkyKey key) { + elements.add(key); size++; } @@ -111,7 +109,7 @@ public void appendSingleton(T t) { *

The caller must ensure that the new group is duplicate-free and does not contain any * elements which are already present. */ - public void appendGroup(ImmutableList group) { + public void appendGroup(ImmutableList group) { addGroup(group, elements); size += group.size(); } @@ -120,7 +118,7 @@ public void appendGroup(ImmutableList group) { * Removes the elements in toRemove from this list. Takes time proportional to the size of the * list, so should not be called often. */ - public void remove(Set toRemove) { + public void remove(Set toRemove) { if (!toRemove.isEmpty()) { size = removeAndGetNewSize(elements, toRemove); } @@ -168,13 +166,13 @@ private static int removeAndGetNewSize(List elements, Set toRemove) { } /** Returns the group at position {@code i}. {@code i} must be less than {@link #listSize()}. */ - @SuppressWarnings("unchecked") // Cast of Object to List or T. - public ImmutableList get(int i) { + @SuppressWarnings("unchecked") // Cast of Object to List. + public ImmutableList get(int i) { Object obj = elements.get(i); if (obj instanceof List) { - return (ImmutableList) obj; + return (ImmutableList) obj; } - return ImmutableList.of((T) obj); + return ImmutableList.of((SkyKey) obj); } /** Returns the number of groups in this list. */ @@ -205,7 +203,7 @@ public static int numElements(@Compressed Object compressed) { return 1; } - /** Returns the number of groups in a compressed {@code GroupedList}. */ + /** Returns the number of groups in the given compressed {@code GroupedDeps}. */ public static int numGroups(@Compressed Object compressed) { if (compressed == EMPTY_LIST) { return 0; @@ -217,19 +215,17 @@ public static int numGroups(@Compressed Object compressed) { } /** - * Expands a compressed {@code GroupedList} into an {@link Iterable}. Equivalent to {@link - * #getAllElementsAsIterable()} but potentially more efficient. + * Expands a compressed {@code GroupedDeps} into an {@link Iterable}. Equivalent to {@link + * #getAllElementsAsIterable} but potentially more efficient. */ - @SuppressWarnings("unchecked") - public static Iterable compressedToIterable(@Compressed Object compressed) { + public static Iterable compressedToIterable(@Compressed Object compressed) { if (compressed == EMPTY_LIST) { return ImmutableList.of(); } if (compressed.getClass().isArray()) { - return GroupedList.create(compressed).getAllElementsAsIterable(); + return GroupedDeps.create(compressed).getAllElementsAsIterable(); } - checkState(!(compressed instanceof List), compressed); - return ImmutableList.of((T) compressed); + return ImmutableList.of((SkyKey) compressed); } /** @@ -238,7 +234,7 @@ public static Iterable compressedToIterable(@Compressed Object compressed *

This method should only be used when it is not possible to enforce the type via annotations. */ public static @Compressed Object castAsCompressed(Object obj) { - checkArgument(!(obj instanceof GroupedList), obj); + checkArgument(!(obj instanceof GroupedDeps), obj); return (@Compressed Object) obj; } @@ -252,7 +248,7 @@ public boolean isEmpty() { * Call {@link #toSet} instead and use the result if doing multiple contains checks and this is * not a {@link WithHashSet}. */ - public boolean contains(T needle) { + public boolean contains(SkyKey needle) { return contains(elements, needle); } @@ -284,13 +280,13 @@ private static boolean contains(List elements, Object needle) { } @SuppressWarnings("unchecked") - public ImmutableSet toSet() { - ImmutableSet.Builder builder = ImmutableSet.builderWithExpectedSize(size); + public ImmutableSet toSet() { + ImmutableSet.Builder builder = ImmutableSet.builderWithExpectedSize(size); for (Object obj : elements) { if (obj instanceof List) { - builder.addAll((List) obj); + builder.addAll((List) obj); } else { - builder.add((T) obj); + builder.add((SkyKey) obj); } } return builder.build(); @@ -300,9 +296,9 @@ private static int sizeOf(Object obj) { return obj instanceof List ? ((List) obj).size() : 1; } - public static GroupedList create(@Compressed Object compressed) { + public static GroupedDeps create(@Compressed Object compressed) { if (compressed == EMPTY_LIST) { - return new GroupedList<>(); + return new GroupedDeps(); } if (compressed.getClass().isArray()) { int size = 0; @@ -310,10 +306,10 @@ public static GroupedList create(@Compressed Object compressed) { for (Object item : compressedArray) { size += sizeOf(item); } - return new GroupedList<>(size, compressedArray); + return new GroupedDeps(size, compressedArray); } // Just a single element. - return new GroupedList<>(1, new Object[] {compressed}); + return new GroupedDeps(1, new Object[] {compressed}); } @Override @@ -337,16 +333,16 @@ private static boolean checkUnorderedEqualityWithoutDuplicates(List first, Li } /** - * A grouping-unaware view of a {@code GroupedList} which does not support modifications. + * A grouping-unaware view which does not support modifications. * *

This is implemented as a {@code Collection} so that calling {@link Iterables#size} on the * return value of {@link #getAllElementsAsIterable} will take constant time. */ - private final class CollectionView extends AbstractCollection { + private final class CollectionView extends AbstractCollection { @Override - public Iterator iterator() { - return new UngroupedIterator<>(elements); + public Iterator iterator() { + return new UngroupedIterator(elements); } @Override @@ -356,10 +352,10 @@ public int size() { } /** An iterator that loops through every element in each group. */ - private static final class UngroupedIterator implements Iterator { + private static final class UngroupedIterator implements Iterator { private final List elements; private int outerIndex = 0; - @Nullable private List currentGroup; + @Nullable private List currentGroup; private int innerIndex = 0; private UngroupedIterator(List elements) { @@ -371,23 +367,23 @@ public boolean hasNext() { return outerIndex < elements.size(); } - @SuppressWarnings("unchecked") // Cast of Object to List or T. + @SuppressWarnings("unchecked") // Cast of Object to List. @Override - public T next() { + public SkyKey next() { if (currentGroup != null) { return nextFromCurrentGroup(); } Object next = elements.get(outerIndex); if (next instanceof List) { - currentGroup = (List) next; + currentGroup = (List) next; innerIndex = 0; return nextFromCurrentGroup(); } - return (T) elements.get(outerIndex++); + return (SkyKey) elements.get(outerIndex++); } - private T nextFromCurrentGroup() { - T next = currentGroup.get(innerIndex++); + private SkyKey nextFromCurrentGroup() { + SkyKey next = currentGroup.get(innerIndex++); if (innerIndex == currentGroup.size()) { outerIndex++; currentGroup = null; @@ -397,7 +393,7 @@ private T nextFromCurrentGroup() { } @ThreadHostile - public Collection getAllElementsAsIterable() { + public Collection getAllElementsAsIterable() { return collectionView; } @@ -406,10 +402,10 @@ public boolean equals(Object other) { if (this == other) { return true; } - if (!(other instanceof GroupedList)) { + if (!(other instanceof GroupedDeps)) { return false; } - GroupedList that = (GroupedList) other; + GroupedDeps that = (GroupedDeps) other; // We must check the deps, ignoring the ordering of deps in the same group. if (this.size != that.size || this.elements.size() != that.elements.size()) { return false; @@ -445,7 +441,7 @@ public String toString() { * iterator is needed here because, to optimize memory, we store single-element lists as elements * internally, and so they must be wrapped before they're returned. */ - private class GroupedIterator implements Iterator> { + private class GroupedIterator implements Iterator> { private final Iterator iter = elements.iterator(); @Override @@ -453,19 +449,19 @@ public boolean hasNext() { return iter.hasNext(); } - @SuppressWarnings("unchecked") // Cast of Object to List or T. + @SuppressWarnings("unchecked") // Cast of Object to List or T. @Override - public List next() { + public List next() { Object obj = iter.next(); if (obj instanceof List) { - return (List) obj; + return (List) obj; } - return ImmutableList.of((T) obj); + return ImmutableList.of((SkyKey) obj); } } @Override - public Iterator> iterator() { + public Iterator> iterator() { return new GroupedIterator(); } @@ -494,37 +490,37 @@ private static void addGroup(ImmutableList group, List elements) { } /** - * A {@link GroupedList} which keeps a {@link HashSet} of its elements up to date, resulting in a + * A {@link GroupedDeps} which keeps a {@link HashSet} of its elements up to date, resulting in a * higher memory cost and faster {@link #contains} operations. */ - public static class WithHashSet extends GroupedList { - private final HashSet set = new HashSet<>(); + public static class WithHashSet extends GroupedDeps { + private final HashSet set = new HashSet<>(); @Override - public void appendSingleton(T t) { - super.appendSingleton(t); - set.add(t); + public void appendSingleton(SkyKey key) { + super.appendSingleton(key); + set.add(key); } @Override - public void appendGroup(ImmutableList group) { + public void appendGroup(ImmutableList group) { super.appendGroup(group); set.addAll(group); } @Override - public void remove(Set toRemove) { + public void remove(Set toRemove) { super.remove(toRemove); set.removeAll(toRemove); } @Override - public boolean contains(T needle) { + public boolean contains(SkyKey needle) { return set.contains(needle); } @Override - public ImmutableSet toSet() { + public ImmutableSet toSet() { return ImmutableSet.copyOf(set); } } diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java index b06cb64c0941d1..7b40388d243521 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryNodeEntry.java @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.KeyToConsolidate.Op; import com.google.errorprone.annotations.ForOverride; import java.util.ArrayList; @@ -80,10 +79,10 @@ public class InMemoryNodeEntry implements NodeEntry { /** * This object represents the direct deps of the node, in groups if the {@code SkyFunction} - * requested them that way. It contains either the in-progress direct deps, stored as a {@code - * GroupedList} (constructed via {@link GroupedList.WithHashSet} if {@code + * requested them that way. It contains either the in-progress direct deps, stored as a {@link + * GroupedDeps} (constructed via {@link GroupedDeps.WithHashSet} if {@code * key.supportsPartialReevaluation()}) before the node is finished building, or the full direct - * deps, compressed in a memory-efficient way (via {@link GroupedList#compress}, after the node is + * deps, compressed in a memory-efficient way (via {@link GroupedDeps#compress}, after the node is * done. * *

It is initialized lazily in getTemporaryDirectDeps() to save a little memory. @@ -210,24 +209,24 @@ public SkyValue toValue() { @Override public Iterable getDirectDeps() { - return GroupedList.compressedToIterable(getCompressedDirectDepsForDoneEntry()); + return GroupedDeps.compressedToIterable(getCompressedDirectDepsForDoneEntry()); } @Override public boolean hasAtLeastOneDep() { - return GroupedList.numGroups(getCompressedDirectDepsForDoneEntry()) > 0; + return GroupedDeps.numGroups(getCompressedDirectDepsForDoneEntry()) > 0; } - /** Returns the compressed {@link GroupedList} of direct deps. Can only be called when done. */ - public final synchronized @GroupedList.Compressed Object getCompressedDirectDepsForDoneEntry() { + /** Returns the compressed {@link GroupedDeps} of direct deps. Can only be called when done. */ + public final synchronized @GroupedDeps.Compressed Object getCompressedDirectDepsForDoneEntry() { assertKeepEdges(); checkState(isDone(), "no deps until done. NodeEntry: %s", this); checkNotNull(directDeps, "deps can't be null: %s", this); - return GroupedList.castAsCompressed(directDeps); + return GroupedDeps.castAsCompressed(directDeps); } public int getNumDirectDeps() { - return GroupedList.numElements(getCompressedDirectDepsForDoneEntry()); + return GroupedDeps.numElements(getCompressedDirectDepsForDoneEntry()); } @Override @@ -438,11 +437,11 @@ private void assertKeepEdges() { */ @ForOverride protected DirtyBuildingState createDirtyBuildingStateForDoneNode( - DirtyType dirtyType, GroupedList directDeps, SkyValue value) { + DirtyType dirtyType, GroupedDeps directDeps, SkyValue value) { return DirtyBuildingState.create(dirtyType, directDeps, value, key.hasLowFanout()); } - private static final GroupedList EMPTY_LIST = new GroupedList<>(); + private static final GroupedDeps EMPTY_LIST = new GroupedDeps(); @Nullable @Override @@ -452,14 +451,14 @@ public synchronized MarkedDirtyResult markDirty(DirtyType dirtyType) { assertKeepEdges(); } if (isDone()) { - GroupedList directDeps = - keepsEdges() ? GroupedList.create(getCompressedDirectDepsForDoneEntry()) : EMPTY_LIST; + GroupedDeps directDeps = + keepsEdges() ? GroupedDeps.create(getCompressedDirectDepsForDoneEntry()) : EMPTY_LIST; dirtyBuildingState = createDirtyBuildingStateForDoneNode(dirtyType, directDeps, value); value = null; this.directDeps = null; return new MarkedDirtyResult( keepsEdges() - ? ReverseDepsUtility.getReverseDeps(this, /*checkConsistency=*/ true) + ? ReverseDepsUtility.getReverseDeps(this, /* checkConsistency= */ true) : ImmutableList.of()); } if (dirtyType.equals(DirtyType.FORCE_REBUILD)) { @@ -539,7 +538,7 @@ public synchronized Iterable getAllDirectDepsForIncompleteNode() } else { // There may be duplicates here. Make sure everything is unique. ImmutableSet.Builder result = ImmutableSet.builder(); - for (Iterable group : getTemporaryDirectDeps()) { + for (List group : getTemporaryDirectDeps()) { result.addAll(group); } result.addAll(dirtyBuildingState.getAllRemainingDirtyDirectDeps(/*preservePosition=*/ false)); @@ -567,9 +566,8 @@ public synchronized void markRebuilding() { checkNotNull(dirtyBuildingState, this).markRebuilding(); } - @SuppressWarnings("unchecked") @Override - public synchronized GroupedList getTemporaryDirectDeps() { + public synchronized GroupedDeps getTemporaryDirectDeps() { checkState(!isDone(), "temporary shouldn't be done: %s", this); if (directDeps == null) { // Initialize lazily, to save a little memory. @@ -577,9 +575,9 @@ public synchronized GroupedList getTemporaryDirectDeps() { // If the key opts into partial reevaluation, tracking deps with a HashSet is worth the extra // memory cost -- see SkyFunctionEnvironment.PartialReevaluation. directDeps = - key.supportsPartialReevaluation() ? new GroupedList.WithHashSet<>() : new GroupedList<>(); + key.supportsPartialReevaluation() ? new GroupedDeps.WithHashSet() : new GroupedDeps(); } - return (GroupedList) directDeps; + return (GroupedDeps) directDeps; } final synchronized int getNumTemporaryDirectDeps() { @@ -637,7 +635,7 @@ public void addTemporaryDirectDepsInGroups(Set deps, List group } return; } - GroupedList temporaryDirectDeps = getTemporaryDirectDeps(); + GroupedDeps temporaryDirectDeps = getTemporaryDirectDeps(); Iterator it = deps.iterator(); synchronized (this) { temporaryDirectDeps.ensureCapacityForAdditionalGroups(groupSizes.size()); @@ -710,7 +708,7 @@ protected synchronized MoreObjects.ToStringHelper toStringHelper() { .add( "directDeps", isDone() && keepsEdges() - ? GroupedList.create(getCompressedDirectDepsForDoneEntry()) + ? GroupedDeps.create(getCompressedDirectDepsForDoneEntry()) : directDeps) .add("reverseDeps", ReverseDepsUtility.toString(this)) .add("dirtyBuildingState", dirtyBuildingState); diff --git a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java index b9bfd6665b758d..23f5fdce799d64 100644 --- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java +++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java @@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; -import com.google.devtools.build.lib.util.GroupedList; import java.util.List; import java.util.Set; import javax.annotation.Nullable; @@ -494,11 +493,11 @@ default Version getMaxTransitiveSourceVersion() { void markRebuilding(); /** - * Returns the {@link GroupedList} of direct dependencies. This may only be called while the node + * Returns the {@link GroupedDeps} of direct dependencies. This may only be called while the node * is being evaluated (i.e. before {@link #setValue} and after {@link #markDirty}. */ @ThreadSafe - GroupedList getTemporaryDirectDeps(); + GroupedDeps getTemporaryDirectDeps(); @ThreadSafe boolean noDepsLastBuild(); diff --git a/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java b/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java index a58c8a2cac1bbc..7d69c6840feaa4 100644 --- a/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java +++ b/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java @@ -17,7 +17,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.supplier.InterruptibleSupplier; import com.google.devtools.build.lib.supplier.MemoizingInterruptibleSupplier; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.NodeEntry.DirtyType; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.Map; @@ -124,11 +123,12 @@ default InterruptibleSupplier getBatchAsync( * @param previouslyRequestedDeps deps that have already been requested during this build and * should not be prefetched because they will be subsequently fetched anyway * @return {@code previouslyRequestedDeps} as a set if the implementation called {@link - * GroupedList#toSet} (so that the caller may reuse it), otherwise {@code null} + * GroupedDeps#toSet} (so that the caller may reuse it), otherwise {@code null} */ + @CanIgnoreReturnValue @Nullable default ImmutableSet prefetchDeps( - SkyKey requestor, Set oldDeps, GroupedList previouslyRequestedDeps) + SkyKey requestor, Set oldDeps, GroupedDeps previouslyRequestedDeps) throws InterruptedException { return null; } diff --git a/src/main/java/com/google/devtools/build/skyframe/RecordingSkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/RecordingSkyFunctionEnvironment.java index 120fc4a1bc56cf..a56cada008e571 100644 --- a/src/main/java/com/google/devtools/build/skyframe/RecordingSkyFunctionEnvironment.java +++ b/src/main/java/com/google/devtools/build/skyframe/RecordingSkyFunctionEnvironment.java @@ -15,7 +15,6 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.events.ExtendedEventHandler; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.SkyFunction.Environment; import java.util.function.Consumer; import java.util.function.Supplier; @@ -158,7 +157,7 @@ public boolean inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors() { @Nullable @Override - public GroupedList getTemporaryDirectDeps() { + public GroupedDeps getTemporaryDirectDeps() { return delegate.getTemporaryDirectDeps(); } diff --git a/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java b/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java index eeba0e398beb03..e75ee88cb2c919 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java +++ b/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.profiler.AutoProfiler; import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.QueryableGraph.Reason; import com.google.devtools.build.skyframe.SkyFunctionEnvironment.UndonePreviouslyRequestedDeps; import java.time.Duration; @@ -147,7 +146,7 @@ private static ErrorInfo checkForCycles(SkyKey root, ParallelEvaluatorContext ev continue; } maybeMarkRebuilding(entry); - GroupedList directDeps = entry.getTemporaryDirectDeps(); + GroupedDeps directDeps = entry.getTemporaryDirectDeps(); // Find out which children have errors. Similar logic to that in Evaluate#run(). List errorDeps = getChildrenErrorsForCycle( @@ -271,7 +270,7 @@ private static ErrorInfo checkForCycles(SkyKey root, ParallelEvaluatorContext ev } // This node is not yet known to be in a cycle. So process its children. - GroupedList temporaryDirectDeps = entry.getTemporaryDirectDeps(); + GroupedDeps temporaryDirectDeps = entry.getTemporaryDirectDeps(); if (temporaryDirectDeps.isEmpty()) { continue; } @@ -452,7 +451,7 @@ private static Set removeDescendantsOfCycleValue( int cycleLength, ParallelEvaluatorContext evaluatorContext) throws InterruptedException { - GroupedList directDeps = entry.getTemporaryDirectDeps(); + GroupedDeps directDeps = entry.getTemporaryDirectDeps(); Set unvisitedDeps = Sets.newHashSet(directDeps.getAllElementsAsIterable()); unvisitedDeps.remove(cycleChild); // Remove any children from this node that are not part of the cycle we just found. They are diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java index 58526cbcd82fd3..e743559726f32c 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.Reportable; -import com.google.devtools.build.lib.util.GroupedList; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; @@ -281,7 +280,7 @@ SkyframeLookupResult getValuesAndExceptions(Iterable depKeys) * you're doing! */ @Nullable - default GroupedList getTemporaryDirectDeps() { + default GroupedDeps getTemporaryDirectDeps() { return null; } diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java index 3ca363125c4ee9..aff9b56da910a1 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java @@ -33,7 +33,6 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.Reportable; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationState; import com.google.devtools.build.skyframe.NodeEntry.DependencyState; import com.google.devtools.build.skyframe.QueryableGraph.LookupHint; @@ -68,7 +67,7 @@ class SkyFunctionEnvironment extends AbstractSkyFunctionEnvironment private boolean building = true; private SkyKey depErrorKey = null; private final SkyKey skyKey; - private final GroupedList previouslyRequestedDeps; + private final GroupedDeps previouslyRequestedDeps; /** * The deps requested during the previous build of this node. Used for two reasons: (1) They are @@ -154,7 +153,7 @@ class SkyFunctionEnvironment extends AbstractSkyFunctionEnvironment static SkyFunctionEnvironment create( SkyKey skyKey, - GroupedList previouslyRequestedDeps, + GroupedDeps previouslyRequestedDeps, Set oldDeps, ParallelEvaluatorContext evaluatorContext) throws InterruptedException, UndonePreviouslyRequestedDeps { @@ -174,7 +173,7 @@ static SkyFunctionEnvironment create( static SkyFunctionEnvironment createForError( SkyKey skyKey, - GroupedList previouslyRequestedDeps, + GroupedDeps previouslyRequestedDeps, Map bubbleErrorInfo, Set oldDeps, ParallelEvaluatorContext evaluatorContext) @@ -194,7 +193,7 @@ static SkyFunctionEnvironment createForError( private SkyFunctionEnvironment( SkyKey skyKey, - GroupedList previouslyRequestedDeps, + GroupedDeps previouslyRequestedDeps, @Nullable Map bubbleErrorInfo, Set oldDeps, ParallelEvaluatorContext evaluatorContext, @@ -295,7 +294,7 @@ NestedSet reportEventsAndGetEventsToStore(NodeEntry entry, boolean e return NestedSetBuilder.emptySet(Order.STABLE_ORDER); } - GroupedList depKeys = entry.getTemporaryDirectDeps(); + GroupedDeps depKeys = entry.getTemporaryDirectDeps(); if (eventsToReport.isEmpty() && depKeys.isEmpty()) { return NestedSetBuilder.emptySet(Order.STABLE_ORDER); } @@ -755,7 +754,7 @@ public ExtendedEventHandler getListener() { } @Override - public GroupedList getTemporaryDirectDeps() { + public GroupedDeps getTemporaryDirectDeps() { return previouslyRequestedDeps; } @@ -886,7 +885,7 @@ Set commitAndGetParents(NodeEntry primaryEntry) throws InterruptedExcept } else { valueWithMetadata = ValueWithMetadata.normal(value, errorInfo, events); } - GroupedList temporaryDirectDeps = primaryEntry.getTemporaryDirectDeps(); + GroupedDeps temporaryDirectDeps = primaryEntry.getTemporaryDirectDeps(); if (!oldDeps.isEmpty()) { // Remove the rdep on this entry for each of its old deps that is no longer a direct dep. Set depsToRemove = Sets.difference(oldDeps, temporaryDirectDeps.toSet()); @@ -1078,7 +1077,7 @@ private static final class PartialReevaluation extends SkyFunctionEnvironment { private PartialReevaluation( SkyKey skyKey, - GroupedList previouslyRequestedDeps, + GroupedDeps previouslyRequestedDeps, Set oldDeps, ParallelEvaluatorContext evaluatorContext) throws UndonePreviouslyRequestedDeps, InterruptedException { diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunctionTest.java index 73033aaf3e9c43..fea4f75911f7b7 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunctionTest.java @@ -26,8 +26,8 @@ import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.skyframe.TargetLoadingUtil.TargetAndErrorIfAny; import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.EvaluationResult; +import com.google.devtools.build.skyframe.GroupedDeps; import com.google.devtools.build.skyframe.SimpleSkyframeLookupResult; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyKey; @@ -51,8 +51,8 @@ public void noRepeatedLabelVisitationForTransitiveTraversalFunction() throws Exc Package pkg = loadPackage(label.getPackageIdentifier()); TargetAndErrorIfAny targetAndErrorIfAny = new TargetAndErrorIfAny( - /*packageLoadedSuccessfully=*/ true, - /*errorLoadingTarget=*/ null, + /* packageLoadedSuccessfully= */ true, + /* errorLoadingTarget= */ null, pkg.getTarget(label.getName())); TransitiveTraversalFunction function = new TransitiveTraversalFunction() { @@ -61,21 +61,21 @@ TargetAndErrorIfAny loadTarget(Environment env, Label label) { return targetAndErrorIfAny; } }; - // Create the GroupedList saying we had already requested two targets the last time we called + // Create the GroupedDeps saying we had already requested two targets the last time we called // #compute. - GroupedList groupedList = new GroupedList<>(); - groupedList.appendSingleton(PackageValue.key(label.getPackageIdentifier())); + GroupedDeps groupedDeps = new GroupedDeps(); + groupedDeps.appendSingleton(PackageValue.key(label.getPackageIdentifier())); // Note that these targets don't actually exist in the package we created initially. It doesn't // matter for the purpose of this test, the original package was just to create some objects // that we needed. SkyKey fakeDep1 = function.getKey(Label.parseCanonical("//foo:bar")); SkyKey fakeDep2 = function.getKey(Label.parseCanonical("//foo:baz")); - groupedList.appendGroup(ImmutableList.of(fakeDep1, fakeDep2)); + groupedDeps.appendGroup(ImmutableList.of(fakeDep1, fakeDep2)); AtomicBoolean wasOptimizationUsed = new AtomicBoolean(false); SkyFunction.Environment mockEnv = Mockito.mock(SkyFunction.Environment.class); - when(mockEnv.getTemporaryDirectDeps()).thenReturn(groupedList); - when(mockEnv.getValuesAndExceptions(groupedList.get(1))) + when(mockEnv.getTemporaryDirectDeps()).thenReturn(groupedDeps); + when(mockEnv.getValuesAndExceptions(groupedDeps.get(1))) .thenAnswer( (invocationOnMock) -> { wasOptimizationUsed.set(true); diff --git a/src/test/java/com/google/devtools/build/lib/util/GroupedListTest.java b/src/test/java/com/google/devtools/build/lib/util/GroupedListTest.java deleted file mode 100644 index 03179c0fdfcbb9..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/util/GroupedListTest.java +++ /dev/null @@ -1,246 +0,0 @@ -// Copyright 2014 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.util; - -import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertThrows; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; -import com.google.testing.junit.testparameterinjector.TestParameter; -import com.google.testing.junit.testparameterinjector.TestParameterInjector; -import java.util.List; -import java.util.stream.IntStream; -import org.junit.Test; -import org.junit.runner.RunWith; - -/** Tests for {@link GroupedList}. */ -@RunWith(TestParameterInjector.class) -public final class GroupedListTest { - - @TestParameter private boolean withHashSet; - - private GroupedList createGroupedList() { - if (withHashSet) { - return new GroupedList.WithHashSet<>(); - } - return new GroupedList<>(); - } - - @Test - public void singleGroup(@TestParameter({"0", "1", "2", "10"}) int size) { - GroupedList groupedList = createGroupedList(); - ImmutableList elements = - IntStream.range(0, size).mapToObj(String::valueOf).collect(toImmutableList()); - groupedList.appendGroup(elements); - checkGroups(groupedList, size == 0 ? ImmutableList.of() : ImmutableList.of(elements)); - } - - @Test - public void appendEmptyGroup_noOp() { - GroupedList groupedList = createGroupedList(); - groupedList.appendGroup(ImmutableList.of()); - assertThat(groupedList.isEmpty()).isTrue(); - groupedList.appendSingleton("a"); - groupedList.appendGroup(ImmutableList.of()); - groupedList.appendSingleton("b"); - checkGroups(groupedList, ImmutableList.of(ImmutableList.of("a"), ImmutableList.of("b"))); - } - - @Test - public void identical_equal() { - GroupedList abc1 = createGroupedList(); - GroupedList abc2 = createGroupedList(); - abc1.appendGroup(ImmutableList.of("a", "b", "c")); - abc2.appendGroup(ImmutableList.of("a", "b", "c")); - assertThat(abc1).isEqualTo(abc2); - } - - @Test - public void appendSingletonAndAppendGroupSizeOne_equal() { - GroupedList aSingleton = createGroupedList(); - GroupedList aGroup = createGroupedList(); - aSingleton.appendSingleton("a"); - aGroup.appendGroup(ImmutableList.of("a")); - assertThat(aSingleton).isEqualTo(aGroup); - } - - @Test - public void differentOrderWithinGroup_equal() { - GroupedList ab = createGroupedList(); - GroupedList ba = createGroupedList(); - ab.appendGroup(ImmutableList.of("a", "b")); - ba.appendGroup(ImmutableList.of("b", "a")); - assertThat(ab).isEqualTo(ba); - } - - @Test - public void differentElements_notEqual() { - GroupedList abc = createGroupedList(); - GroupedList xyz = createGroupedList(); - abc.appendGroup(ImmutableList.of("a", "b", "c")); - xyz.appendGroup(ImmutableList.of("x", "y", "z")); - assertThat(abc).isNotEqualTo(xyz); - } - - @Test - public void differentOrderOfGroups_notEqual() { - GroupedList ab = createGroupedList(); - GroupedList ba = createGroupedList(); - ab.appendSingleton("a"); - ab.appendSingleton("b"); - ba.appendSingleton("b"); - ba.appendSingleton("a"); - assertThat(ab).isNotEqualTo(ba); - } - - @Test - public void differentGroupings_notEqual() { - GroupedList abGroup = createGroupedList(); - GroupedList abSingletons = createGroupedList(); - abGroup.appendGroup(ImmutableList.of("a", "b")); - abSingletons.appendSingleton("a"); - abSingletons.appendSingleton("b"); - assertThat(abGroup).isNotEqualTo(abSingletons); - } - - @Test - public void groups() { - GroupedList groupedList = createGroupedList(); - ImmutableList> groups = - ImmutableList.of( - ImmutableList.of("1"), - ImmutableList.of("2a", "2b"), - ImmutableList.of("3"), - ImmutableList.of("4"), - ImmutableList.of("5a", "5b", "5c"), - ImmutableList.of("6a", "6b", "6c")); - groups.forEach(groupedList::appendGroup); - checkGroups(groupedList, groups); - } - - @Test - public void remove_groupsIntact() { - GroupedList groupedList = createGroupedList(); - groupedList.appendGroup(ImmutableList.of("1a", "1b")); - groupedList.appendGroup(ImmutableList.of("2a", "2b", "2c")); - groupedList.appendGroup(ImmutableList.of("3a", "3b")); - - groupedList.remove(ImmutableSet.of("2c")); - - checkGroups( - groupedList, - ImmutableList.of( - ImmutableList.of("1a", "1b"), - ImmutableList.of("2a", "2b"), - ImmutableList.of("3a", "3b"))); - } - - @Test - public void remove_groupBecomesSingleton() { - GroupedList groupedList = createGroupedList(); - groupedList.appendGroup(ImmutableList.of("1a", "1b")); - groupedList.appendGroup(ImmutableList.of("2a", "2b", "2c")); - groupedList.appendGroup(ImmutableList.of("3a", "3b")); - - groupedList.remove(ImmutableSet.of("2b", "2c")); - - checkGroups( - groupedList, - ImmutableList.of( - ImmutableList.of("1a", "1b"), ImmutableList.of("2a"), ImmutableList.of("3a", "3b"))); - } - - @Test - public void remove_groupBecomesEmpty() { - GroupedList groupedList = createGroupedList(); - groupedList.appendGroup(ImmutableList.of("1a", "1b")); - groupedList.appendGroup(ImmutableList.of("2a", "2b", "2c")); - groupedList.appendGroup(ImmutableList.of("3a", "3b")); - - groupedList.remove(ImmutableSet.of("2a", "2b", "2c")); - - checkGroups( - groupedList, ImmutableList.of(ImmutableList.of("1a", "1b"), ImmutableList.of("3a", "3b"))); - } - - @Test - public void remove_singleton() { - GroupedList groupedList = createGroupedList(); - groupedList.appendGroup(ImmutableList.of("1a", "1b")); - groupedList.appendSingleton("2"); - groupedList.appendGroup(ImmutableList.of("3a", "3b")); - - groupedList.remove(ImmutableSet.of("2")); - - checkGroups( - groupedList, ImmutableList.of(ImmutableList.of("1a", "1b"), ImmutableList.of("3a", "3b"))); - } - - @Test - public void remove_wholeGroupedListBecomesEmpty() { - GroupedList groupedList = createGroupedList(); - groupedList.appendGroup(ImmutableList.of("1a", "1b")); - groupedList.appendGroup(ImmutableList.of("2a", "2b", "2c")); - groupedList.appendGroup(ImmutableList.of("3a", "3b")); - - groupedList.remove(ImmutableSet.of("1a", "1b", "2a", "2b", "2c", "3a", "3b")); - - checkGroups(groupedList, ImmutableList.of()); - } - - @Test - public void remove_elementNotPresent_throws() { - GroupedList groupedList = createGroupedList(); - groupedList.appendGroup(ImmutableList.of("1a", "1b")); - groupedList.appendGroup(ImmutableList.of("2a", "2b", "2c")); - groupedList.appendGroup(ImmutableList.of("3a", "3b")); - - assertThrows(RuntimeException.class, () -> groupedList.remove(ImmutableSet.of("2c", "2d"))); - } - - private static void checkGroups( - GroupedList groupedList, List> expectedGroups) { - assertThat(groupedList.isEmpty()).isEqualTo(expectedGroups.isEmpty()); - assertThat(groupedList.listSize()).isEqualTo(expectedGroups.size()); - assertThat(groupedList).containsExactlyElementsIn(expectedGroups).inOrder(); - - ImmutableList expectedFlattened = - ImmutableList.copyOf(Iterables.concat(expectedGroups)); - assertThat(groupedList.numElements()).isEqualTo(expectedFlattened.size()); - assertThat(groupedList.getAllElementsAsIterable()) - .containsExactlyElementsIn(expectedFlattened) - .inOrder(); - if (groupedList instanceof GroupedList.WithHashSet) { - assertThat(groupedList.toSet()).containsExactlyElementsIn(expectedFlattened); - } else { - assertThat(groupedList.toSet()).containsExactlyElementsIn(expectedFlattened).inOrder(); - } - - checkCompression(groupedList); - } - - private static void checkCompression(GroupedList groupedList) { - @GroupedList.Compressed Object compressed = groupedList.compress(); - assertThat(GroupedList.numElements(compressed)).isEqualTo(groupedList.numElements()); - assertThat(GroupedList.numGroups(compressed)).isEqualTo(groupedList.listSize()); - assertThat(GroupedList.compressedToIterable(compressed)) - .containsExactlyElementsIn(groupedList.getAllElementsAsIterable()) - .inOrder(); - assertThat(GroupedList.create(compressed)).containsExactlyElementsIn(groupedList).inOrder(); - assertThat(GroupedList.create(compressed)).isNotInstanceOf(GroupedList.WithHashSet.class); - } -} diff --git a/src/test/java/com/google/devtools/build/skyframe/GroupedDepsTest.java b/src/test/java/com/google/devtools/build/skyframe/GroupedDepsTest.java new file mode 100644 index 00000000000000..d5f2c009e46953 --- /dev/null +++ b/src/test/java/com/google/devtools/build/skyframe/GroupedDepsTest.java @@ -0,0 +1,257 @@ +// Copyright 2014 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.skyframe; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.testing.junit.testparameterinjector.TestParameter; +import com.google.testing.junit.testparameterinjector.TestParameterInjector; +import java.util.List; +import java.util.stream.IntStream; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Tests for {@link GroupedDeps}. */ +@RunWith(TestParameterInjector.class) +public final class GroupedDepsTest { + + @TestParameter private boolean withHashSet; + + private GroupedDeps createEmpty() { + if (withHashSet) { + return new GroupedDeps.WithHashSet(); + } + return new GroupedDeps(); + } + + private static SkyKey key(String arg) { + return GraphTester.skyKey(arg); + } + + @Test + public void singleGroup(@TestParameter({"0", "1", "2", "10"}) int size) { + GroupedDeps deps = createEmpty(); + ImmutableList elements = + IntStream.range(0, size).mapToObj(i -> key(String.valueOf(i))).collect(toImmutableList()); + deps.appendGroup(elements); + checkGroups(deps, size == 0 ? ImmutableList.of() : ImmutableList.of(elements)); + } + + @Test + public void appendEmptyGroup_noOp() { + GroupedDeps deps = createEmpty(); + deps.appendGroup(ImmutableList.of()); + assertThat(deps.isEmpty()).isTrue(); + deps.appendSingleton(key("a")); + deps.appendGroup(ImmutableList.of()); + deps.appendSingleton(key("b")); + checkGroups(deps, ImmutableList.of(ImmutableList.of(key("a")), ImmutableList.of(key("b")))); + } + + @Test + public void identical_equal() { + GroupedDeps abc1 = createEmpty(); + GroupedDeps abc2 = createEmpty(); + abc1.appendGroup(ImmutableList.of(key("a"), key("b"), key("c"))); + abc2.appendGroup(ImmutableList.of(key("a"), key("b"), key("c"))); + assertThat(abc1).isEqualTo(abc2); + } + + @Test + public void appendSingletonAndAppendGroupSizeOne_equal() { + GroupedDeps aSingleton = createEmpty(); + GroupedDeps aGroup = createEmpty(); + aSingleton.appendSingleton(key("a")); + aGroup.appendGroup(ImmutableList.of(key("a"))); + assertThat(aSingleton).isEqualTo(aGroup); + } + + @Test + public void differentOrderWithinGroup_equal() { + GroupedDeps ab = createEmpty(); + GroupedDeps ba = createEmpty(); + ab.appendGroup(ImmutableList.of(key("a"), key("b"))); + ba.appendGroup(ImmutableList.of(key("b"), key("a"))); + assertThat(ab).isEqualTo(ba); + } + + @Test + public void differentElements_notEqual() { + GroupedDeps abc = createEmpty(); + GroupedDeps xyz = createEmpty(); + abc.appendGroup(ImmutableList.of(key("a"), key("b"), key("c"))); + xyz.appendGroup(ImmutableList.of(key("x"), key("y"), key("z"))); + assertThat(abc).isNotEqualTo(xyz); + } + + @Test + public void differentOrderOfGroups_notEqual() { + GroupedDeps ab = createEmpty(); + GroupedDeps ba = createEmpty(); + ab.appendSingleton(key("a")); + ab.appendSingleton(key("b")); + ba.appendSingleton(key("b")); + ba.appendSingleton(key("a")); + assertThat(ab).isNotEqualTo(ba); + } + + @Test + public void differentGroupings_notEqual() { + GroupedDeps abGroup = createEmpty(); + GroupedDeps abSingletons = createEmpty(); + abGroup.appendGroup(ImmutableList.of(key("a"), key("b"))); + abSingletons.appendSingleton(key("a")); + abSingletons.appendSingleton(key("b")); + assertThat(abGroup).isNotEqualTo(abSingletons); + } + + @Test + public void groups() { + GroupedDeps deps = createEmpty(); + ImmutableList> groups = + ImmutableList.of( + ImmutableList.of(key("1")), + ImmutableList.of(key("2a"), key("2b")), + ImmutableList.of(key("3")), + ImmutableList.of(key("4")), + ImmutableList.of(key("5a"), key("5b"), key("5c")), + ImmutableList.of(key("6a"), key("6b"), key("6c"))); + groups.forEach(deps::appendGroup); + checkGroups(deps, groups); + } + + @Test + public void remove_groupsIntact() { + GroupedDeps deps = createEmpty(); + deps.appendGroup(ImmutableList.of(key("1a"), key("1b"))); + deps.appendGroup(ImmutableList.of(key("2a"), key("2b"), key("2c"))); + deps.appendGroup(ImmutableList.of(key("3a"), key("3b"))); + + deps.remove(ImmutableSet.of(key("2c"))); + + checkGroups( + deps, + ImmutableList.of( + ImmutableList.of(key("1a"), key("1b")), + ImmutableList.of(key("2a"), key("2b")), + ImmutableList.of(key("3a"), key("3b")))); + } + + @Test + public void remove_groupBecomesSingleton() { + GroupedDeps deps = createEmpty(); + deps.appendGroup(ImmutableList.of(key("1a"), key("1b"))); + deps.appendGroup(ImmutableList.of(key("2a"), key("2b"), key("2c"))); + deps.appendGroup(ImmutableList.of(key("3a"), key("3b"))); + + deps.remove(ImmutableSet.of(key("2b"), key("2c"))); + + checkGroups( + deps, + ImmutableList.of( + ImmutableList.of(key("1a"), key("1b")), + ImmutableList.of(key("2a")), + ImmutableList.of(key("3a"), key("3b")))); + } + + @Test + public void remove_groupBecomesEmpty() { + GroupedDeps deps = createEmpty(); + deps.appendGroup(ImmutableList.of(key("1a"), key("1b"))); + deps.appendGroup(ImmutableList.of(key("2a"), key("2b"), key("2c"))); + deps.appendGroup(ImmutableList.of(key("3a"), key("3b"))); + + deps.remove(ImmutableSet.of(key("2a"), key("2b"), key("2c"))); + + checkGroups( + deps, + ImmutableList.of( + ImmutableList.of(key("1a"), key("1b")), ImmutableList.of(key("3a"), key("3b")))); + } + + @Test + public void remove_singleton() { + GroupedDeps deps = createEmpty(); + deps.appendGroup(ImmutableList.of(key("1a"), key("1b"))); + deps.appendSingleton(key("2")); + deps.appendGroup(ImmutableList.of(key("3a"), key("3b"))); + + deps.remove(ImmutableSet.of(key("2"))); + + checkGroups( + deps, + ImmutableList.of( + ImmutableList.of(key("1a"), key("1b")), ImmutableList.of(key("3a"), key("3b")))); + } + + @Test + public void remove_wholeGroupedDepsBecomesEmpty() { + GroupedDeps deps = createEmpty(); + deps.appendGroup(ImmutableList.of(key("1a"), key("1b"))); + deps.appendGroup(ImmutableList.of(key("2a"), key("2b"), key("2c"))); + deps.appendGroup(ImmutableList.of(key("3a"), key("3b"))); + + deps.remove( + ImmutableSet.of( + key("1a"), key("1b"), key("2a"), key("2b"), key("2c"), key("3a"), key("3b"))); + + checkGroups(deps, ImmutableList.of()); + } + + @Test + public void remove_elementNotPresent_throws() { + GroupedDeps deps = createEmpty(); + deps.appendGroup(ImmutableList.of(key("1a"), key("1b"))); + deps.appendGroup(ImmutableList.of(key("2a"), key("2b"), key("2c"))); + deps.appendGroup(ImmutableList.of(key("3a"), key("3b"))); + + assertThrows(RuntimeException.class, () -> deps.remove(ImmutableSet.of(key("2c"), key("2d")))); + } + + private static void checkGroups(GroupedDeps deps, List> expectedGroups) { + assertThat(deps.isEmpty()).isEqualTo(expectedGroups.isEmpty()); + assertThat(deps.listSize()).isEqualTo(expectedGroups.size()); + assertThat(deps).containsExactlyElementsIn(expectedGroups).inOrder(); + + ImmutableList expectedFlattened = + ImmutableList.copyOf(Iterables.concat(expectedGroups)); + assertThat(deps.numElements()).isEqualTo(expectedFlattened.size()); + assertThat(deps.getAllElementsAsIterable()) + .containsExactlyElementsIn(expectedFlattened) + .inOrder(); + if (deps instanceof GroupedDeps.WithHashSet) { + assertThat(deps.toSet()).containsExactlyElementsIn(expectedFlattened); + } else { + assertThat(deps.toSet()).containsExactlyElementsIn(expectedFlattened).inOrder(); + } + + checkCompression(deps); + } + + private static void checkCompression(GroupedDeps deps) { + @GroupedDeps.Compressed Object compressed = deps.compress(); + assertThat(GroupedDeps.numElements(compressed)).isEqualTo(deps.numElements()); + assertThat(GroupedDeps.numGroups(compressed)).isEqualTo(deps.listSize()); + assertThat(GroupedDeps.compressedToIterable(compressed)) + .containsExactlyElementsIn(deps.getAllElementsAsIterable()) + .inOrder(); + assertThat(GroupedDeps.create(compressed)).containsExactlyElementsIn(deps).inOrder(); + assertThat(GroupedDeps.create(compressed)).isNotInstanceOf(GroupedDeps.WithHashSet.class); + } +} diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java index 6b9299668025d7..b6534fa09a222b 100644 --- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.Reportable; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.NodeEntry.DependencyState; import com.google.devtools.build.skyframe.NodeEntry.DirtyState; import com.google.devtools.build.skyframe.NodeEntry.DirtyType; @@ -84,7 +83,7 @@ public void entryAtStartOfEvaluation() { assertThat(entry.isDirty()).isTrue(); assertThat(entry.isChanged()).isTrue(); assertThat(entry.getTemporaryDirectDeps()).isEmpty(); - assertThat(entry.getTemporaryDirectDeps() instanceof GroupedList.WithHashSet) + assertThat(entry.getTemporaryDirectDeps() instanceof GroupedDeps.WithHashSet) .isEqualTo(isPartialReevaluation); } @@ -243,7 +242,7 @@ public void dirtyLifecycle() throws InterruptedException { assertThat(entry.isReadyToEvaluate()).isTrue(); assertThat(entry.hasUnsignaledDeps()).isFalse(); assertThat(entry.getTemporaryDirectDeps()).isEmpty(); - assertThat(entry.getTemporaryDirectDeps() instanceof GroupedList.WithHashSet) + assertThat(entry.getTemporaryDirectDeps() instanceof GroupedDeps.WithHashSet) .isEqualTo(isPartialReevaluation); SkyKey parent = key("parent"); entry.addReverseDepAndCheckIfDone(parent); @@ -307,7 +306,7 @@ public void forceRebuildLifecycle() throws InterruptedException { assertThat(entry.isDirty()).isTrue(); assertThat(entry.isChanged()).isTrue(); assertThat(entry.isDone()).isFalse(); - assertThat(entry.getTemporaryDirectDeps() instanceof GroupedList.WithHashSet) + assertThat(entry.getTemporaryDirectDeps() instanceof GroupedDeps.WithHashSet) .isEqualTo(isPartialReevaluation); assertThatNodeEntry(entry) @@ -856,7 +855,7 @@ public void getCompressedDirectDepsForDoneEntry() throws InterruptedException { } } entry.setValue(new IntegerValue(42), IntVersion.of(42L), null); - assertThat(GroupedList.create(entry.getCompressedDirectDepsForDoneEntry())) + assertThat(GroupedDeps.create(entry.getCompressedDirectDepsForDoneEntry())) .containsExactlyElementsIn(groupedDirectDeps) .inOrder(); } diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java index d31e3f8850bed3..3f0fa4f1e94452 100644 --- a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java +++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java @@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; -import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.NodeEntry.DirtyType; import com.google.errorprone.annotations.ForOverride; import java.util.Collection; @@ -236,7 +235,7 @@ public void removeReverseDep(SkyKey reverseDep) throws InterruptedException { } @Override - public GroupedList getTemporaryDirectDeps() { + public GroupedDeps getTemporaryDirectDeps() { graphListener.accept(myKey, EventType.GET_TEMPORARY_DIRECT_DEPS, Order.BEFORE, null); return super.getTemporaryDirectDeps(); }