Skip to content

Commit

Permalink
Refactor the generic GroupedList into GroupedDeps, specifically d…
Browse files Browse the repository at this point in the history
…esigned to hold skyframe dependency groups.

`GroupedList` is only ever used as `GroupedList<SkyKey>` 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
  • Loading branch information
justinhorvitz authored and copybara-github committed Feb 27, 2023
1 parent 570e25f commit 9d3b3bf
Show file tree
Hide file tree
Showing 21 changed files with 397 additions and 401 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,7 +104,7 @@ public SkyframeLookupResult getValuesAndExceptions(Iterable<? extends SkyKey> de

@Override
@Nullable
public GroupedList<SkyKey> getTemporaryDirectDeps() {
public GroupedDeps getTemporaryDirectDeps() {
return delegate.getTemporaryDirectDeps();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -137,7 +137,7 @@ public boolean inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors() {

@Nullable
@Override
public GroupedList<SkyKey> getTemporaryDirectDeps() {
public GroupedDeps getTemporaryDirectDeps() {
return delegate.getTemporaryDirectDeps();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -140,7 +140,7 @@ Iterable<SkyKey> getStrictLabelAspectDepKeys(
@Nullable
private static Collection<SkyKey> getDepsAfterLastPackageDep(
SkyFunction.Environment env, int offset) {
GroupedList<SkyKey> temporaryDirectDeps = env.getTemporaryDirectDeps();
GroupedDeps temporaryDirectDeps = env.getTemporaryDirectDeps();
if (temporaryDirectDeps == null) {
return null;
}
Expand All @@ -152,7 +152,7 @@ private static Collection<SkyKey> getDepsAfterLastPackageDep(
return temporaryDirectDeps.get(lastPackageDepIndex + offset);
}

private static int getLastPackageValueIndex(GroupedList<SkyKey> directDeps) {
private static int getLastPackageValueIndex(GroupedDeps directDeps) {
int directDepsNumGroups = directDeps.listSize();
for (int i = directDepsNumGroups - 1; i >= 0; i--) {
List<SkyKey> depGroup = directDeps.get(i);
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/com/google/devtools/build/lib/util/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ java_library(
"Either.java",
"FileHandlerQuerier.java",
"Fingerprint.java",
"GroupedList.java",
"JavaSleeper.java",
"LogHandlerQuerier.java",
"LoggingUtil.java",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -113,7 +112,7 @@ public final void dumpDeps(PrintStream out, Predicate<String> filter)
}
out.println(canonicalizedKey);
if (entry.keepsEdges()) {
GroupedList<SkyKey> 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)) {
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/google/devtools/build/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ package(
SKYFRAME_OBJECT_SRCS = [
"AbstractSkyKey.java",
"FunctionHermeticity.java",
"GroupedDeps.java",
"NodeVersion.java",
"SkyFunctionName.java",
"SkyKey.java",
Expand All @@ -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",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -125,7 +124,7 @@ public void markRebuilding() {
}

@Override
public GroupedList<SkyKey> getTemporaryDirectDeps() {
public GroupedDeps getTemporaryDirectDeps() {
return getDelegate().getTemporaryDirectDeps();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,7 +38,7 @@ public abstract class DirtyBuildingState implements PriorityTracker {

static DirtyBuildingState create(
DirtyType dirtyType,
GroupedList<SkyKey> lastBuildDirectDeps,
GroupedDeps lastBuildDirectDeps,
SkyValue lastBuildValue,
boolean hasLowFanout) {
return new FullDirtyBuildingState(dirtyType, lastBuildDirectDeps, lastBuildValue, hasLowFanout);
Expand Down Expand Up @@ -120,7 +119,7 @@ static DirtyBuildingState createNew(boolean hasLowFanout) {
* <p>Public only for the use of alternative graph implementations.
*/
@Nullable
public abstract GroupedList<SkyKey> getLastBuildDirectDeps() throws InterruptedException;
public abstract GroupedDeps getLastBuildDirectDeps() throws InterruptedException;

/**
* The number of groups of the dependencies requested last time when the node was built.
Expand Down Expand Up @@ -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<SkyKey> directDeps)
throws InterruptedException {
final boolean depsUnchangedFromLastBuild(GroupedDeps directDeps) throws InterruptedException {
checkFinishedBuildingWhenAboutToSetValue();
return getLastBuildDirectDeps().equals(directDeps);
}
Expand Down Expand Up @@ -438,12 +436,12 @@ public String toString() {
}

private static class FullDirtyBuildingState extends DirtyBuildingState {
private final GroupedList<SkyKey> lastBuildDirectDeps;
private final GroupedDeps lastBuildDirectDeps;
private final SkyValue lastBuildValue;

private FullDirtyBuildingState(
DirtyType dirtyType,
GroupedList<SkyKey> lastBuildDirectDeps,
GroupedDeps lastBuildDirectDeps,
SkyValue lastBuildValue,
boolean hasLowFanout) {
super(dirtyType, hasLowFanout);
Expand All @@ -466,7 +464,7 @@ public SkyValue getLastBuildValue() {
}

@Override
public GroupedList<SkyKey> getLastBuildDirectDeps() throws InterruptedException {
public GroupedDeps getLastBuildDirectDeps() throws InterruptedException {
return lastBuildDirectDeps;
}

Expand Down
Loading

0 comments on commit 9d3b3bf

Please sign in to comment.