Skip to content

Commit

Permalink
Delete top-down action caching prototype
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 417499871
  • Loading branch information
Googler authored and Copybara-Service committed Dec 21, 2021
1 parent 2ba95a0 commit 16753d6
Show file tree
Hide file tree
Showing 18 changed files with 19 additions and 694 deletions.
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/BUILD
Expand Up @@ -325,7 +325,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_value_dirtiness_checker",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
"//src/main/java/com/google/devtools/build/lib/skyframe:target_pattern_phase_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:top_down_action_cache",
"//src/main/java/com/google/devtools/build/lib/skyframe:workspace_info",
"//src/main/java/com/google/devtools/build/lib/unix",
"//src/main/java/com/google/devtools/build/lib/util",
Expand Down
Expand Up @@ -15,7 +15,6 @@
package com.google.devtools.build.lib.actionsketch;

import com.google.auto.value.AutoValue;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.protobuf.ByteString;
import java.math.BigInteger;
import java.nio.ByteBuffer;
Expand All @@ -24,25 +23,21 @@
/**
* An {@link ActionSketch} encodes a transitive hash of an action sufficient to associate with it
* the result of executing the action. Therefore, this must include a hash on some upper bound of
* all transitively consumed input files, as well as a transitive hash of all action keys.
* all transitively consumed input files.
*/
@AutoValue
public abstract class ActionSketch implements SkyValue {
public abstract class ActionSketch {
public static final int BIGINTEGER_ENCODED_LENGTH = /*length=*/ 1 + /*payload=*/ 39;
public static final int MAX_BYTES = /*hashes=*/ 2 * BIGINTEGER_ENCODED_LENGTH;

private static final ActionSketch NULL_SKETCH =
ActionSketch.builder()
.setTransitiveSourceHash(null)
.setTransitiveActionLookupHash(null)
.autoBuild();

@Nullable
public abstract HashAndVersion transitiveSourceHash();

@Nullable
public abstract BigInteger transitiveActionLookupHash();

public static Builder builder() {
return new AutoValue_ActionSketch.Builder();
}
Expand All @@ -54,20 +49,13 @@ public static Builder builder() {
public abstract static class Builder {
public abstract Builder setTransitiveSourceHash(HashAndVersion transitiveSourceHash);

public abstract Builder setTransitiveActionLookupHash(BigInteger transitiveActionLookupHash);

@Nullable
abstract HashAndVersion transitiveSourceHash();

@Nullable
abstract BigInteger transitiveActionLookupHash();

abstract ActionSketch autoBuild();

public final ActionSketch build() {
return transitiveSourceHash() == null && transitiveActionLookupHash() == null
? NULL_SKETCH
: autoBuild();
return transitiveSourceHash() == null ? NULL_SKETCH : autoBuild();
}
}

Expand All @@ -79,7 +67,6 @@ public final ByteString toBytes() {

public final void writeTo(ByteBuffer buffer) {
writeNextValue(transitiveSourceHash(), buffer);
writeNextValue(transitiveActionLookupHash(), buffer);
}

private static void writeNextValue(HashAndVersion value, ByteBuffer buffer) {
Expand All @@ -91,36 +78,12 @@ private static void writeNextValue(HashAndVersion value, ByteBuffer buffer) {
}
}

private static void writeNextValue(BigInteger value, ByteBuffer buffer) {
if (value == null) {
buffer.put((byte) -1);
} else {
byte[] bytes = value.toByteArray();
buffer.put((byte) bytes.length).put(bytes);
}
}

public static ActionSketch fromBytes(ByteString inputBytes) {
return fromByteBuffer(inputBytes.asReadOnlyByteBuffer());
}

public static ActionSketch fromByteBuffer(ByteBuffer buffer) {
Builder builder =
builder()
.setTransitiveSourceHash(readNextHashAndVersion(buffer))
.setTransitiveActionLookupHash(readNextBigInteger(buffer));
return builder.build();
}

@Nullable
private static BigInteger readNextBigInteger(ByteBuffer buffer) {
byte length = buffer.get();
if (length < 0) {
return null;
}
byte[] val = new byte[length];
buffer.get(val);
return new BigInteger(1, val);
return builder().setTransitiveSourceHash(readNextHashAndVersion(buffer)).build();
}

@Nullable
Expand Down
Expand Up @@ -300,11 +300,7 @@ public void prepareForExecution(
try (SilentCloseable c =
Profiler.instance().profile("prepareSkyframeActionExecutorForExecution")) {
skyframeExecutor.prepareSkyframeActionExecutorForExecution(
env.getReporter(),
executor,
request,
skyframeBuilder.getActionCacheChecker(),
skyframeBuilder.getTopDownActionCache());
env.getReporter(), executor, request, skyframeBuilder.getActionCacheChecker());
}

// Note that executionProgressReceiver accesses builtTargets concurrently (after wrapping in a
Expand Down Expand Up @@ -886,7 +882,6 @@ private Builder createBuilder(
.setVerboseExplanations(options.verboseExplanations)
.setStoreOutputMetadata(options.actionCacheStoreOutputMetadata)
.build()),
env.getTopDownActionCache(),
request.getPackageOptions().checkOutputFiles
// Do not skip invalidation in case the output tree is empty -- this can happen
// after it's cleaned or corrupted.
Expand Down
Expand Up @@ -53,7 +53,6 @@
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.DetailedException;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.TopDownActionCache;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.DetailedExitCode.DetailedExitCodeComparator;
Expand Down Expand Up @@ -87,23 +86,20 @@ public class SkyframeBuilder implements Builder {
private final MetadataProvider fileCache;
private final ActionInputPrefetcher actionInputPrefetcher;
private final ActionCacheChecker actionCacheChecker;
private final TopDownActionCache topDownActionCache;
private final BugReporter bugReporter;

@VisibleForTesting
public SkyframeBuilder(
SkyframeExecutor skyframeExecutor,
ResourceManager resourceManager,
ActionCacheChecker actionCacheChecker,
TopDownActionCache topDownActionCache,
ModifiedFileSet modifiedOutputFiles,
MetadataProvider fileCache,
ActionInputPrefetcher actionInputPrefetcher,
BugReporter bugReporter) {
this.resourceManager = resourceManager;
this.skyframeExecutor = skyframeExecutor;
this.actionCacheChecker = actionCacheChecker;
this.topDownActionCache = topDownActionCache;
this.modifiedOutputFiles = modifiedOutputFiles;
this.fileCache = fileCache;
this.actionInputPrefetcher = actionInputPrefetcher;
Expand Down Expand Up @@ -182,7 +178,6 @@ public void buildArtifacts(
exclusiveTests,
options,
actionCacheChecker,
topDownActionCache,
executionProgressReceiver,
topLevelArtifactContext);
// progressReceiver is finished, so unsynchronized access to builtTargets is now safe.
Expand Down Expand Up @@ -212,7 +207,6 @@ public void buildArtifacts(
exclusiveTest,
options,
actionCacheChecker,
topDownActionCache,
topLevelArtifactContext);
detailedExitCode =
processResult(
Expand Down Expand Up @@ -399,10 +393,6 @@ ActionCacheChecker getActionCacheChecker() {
return actionCacheChecker;
}

TopDownActionCache getTopDownActionCache() {
return topDownActionCache;
}

MetadataProvider getFileCache() {
return fileCache;
}
Expand Down
Expand Up @@ -35,7 +35,6 @@
import com.google.devtools.build.lib.packages.PackageOverheadEstimator;
import com.google.devtools.build.lib.packages.PackageValidator;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.TopDownActionCache;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.io.OutErr;
Expand Down Expand Up @@ -99,17 +98,6 @@ public FileSystem getFileSystemForBuildArtifacts(FileSystem fileSystem) {
return null;
}

/**
* Returns the {@link TopDownActionCache} used by Bazel. It is an error if more than one module
* returns a top-down action cache. If all modules return null, there will be no top-down caching.
*
* <p>This method will be called at the beginning of Bazel startup (in-between {@link #globalInit}
* and {@link #blazeStartup}).
*/
public TopDownActionCache getTopDownActionCache() {
return null;
}

/** Tuple returned by {@link #getFileSystem}. */
@AutoValue
public abstract static class ModuleFileSystem {
Expand Down
Expand Up @@ -42,7 +42,6 @@
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.TopDownActionCache;
import com.google.devtools.build.lib.skyframe.WorkspaceInfoFromDiff;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.DetailedExitCode;
Expand Down Expand Up @@ -106,7 +105,6 @@ public class CommandEnvironment {
private final Consumer<String> shutdownReasonConsumer;

private OutputService outputService;
private TopDownActionCache topDownActionCache;
private String workspaceName;
private boolean hasSyncedPackageLoading = false;
@Nullable private WorkspaceInfoFromDiff workspaceInfoFromDiff;
Expand Down Expand Up @@ -601,11 +599,6 @@ public WorkspaceInfoFromDiff getWorkspaceInfoFromDiff() {
return workspaceInfoFromDiff;
}

/** Returns the top-down action cache to use, or null. */
public TopDownActionCache getTopDownActionCache() {
return topDownActionCache;
}

public ResourceManager getLocalResourceManager() {
return ResourceManager.instance();
}
Expand Down Expand Up @@ -732,8 +725,6 @@ public void beforeCommand(InvocationPolicy invocationPolicy) throws AbruptExitEx

outputService = null;
BlazeModule outputModule = null;
topDownActionCache = null;
BlazeModule topDownCachingModule = null;
if (command.builds()) {
for (BlazeModule module : runtime.getBlazeModules()) {
OutputService moduleService = module.getOutputService();
Expand All @@ -747,18 +738,6 @@ public void beforeCommand(InvocationPolicy invocationPolicy) throws AbruptExitEx
outputService = moduleService;
outputModule = module;
}

TopDownActionCache moduleCache = module.getTopDownActionCache();
if (moduleCache != null) {
if (topDownActionCache != null) {
throw new IllegalStateException(
String.format(
"More than one module (%s and %s) returns a top down action cache",
module.getClass(), topDownCachingModule.getClass()));
}
topDownActionCache = moduleCache;
topDownCachingModule = module;
}
}
}

Expand Down
Expand Up @@ -58,7 +58,6 @@
import com.google.devtools.build.lib.actions.MissingInputFileException;
import com.google.devtools.build.lib.actions.PackageRootResolver;
import com.google.devtools.build.lib.actions.SpawnMetrics;
import com.google.devtools.build.lib.actionsketch.ActionSketch;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.bugreport.BugReporter;
Expand Down Expand Up @@ -208,19 +207,6 @@ private SkyValue computeInternal(SkyKey skyKey, Environment env)
clientEnv = ImmutableMap.of();
}

ActionSketch sketch = null;
TopDownActionCache topDownActionCache = skyframeActionExecutor.getTopDownActionCache();
if (topDownActionCache != null) {
sketch = (ActionSketch) env.getValue(ActionSketchFunction.key(actionLookupData));
if (sketch == null) {
return null;
}
ActionExecutionValue actionExecutionValue = topDownActionCache.get(sketch, actionLookupData);
if (actionExecutionValue != null) {
return actionExecutionValue.transformForSharedAction(action);
}
}

// For restarts of this ActionExecutionFunction we use a ContinuationState variable, below, to
// avoid redoing work.
//
Expand Down Expand Up @@ -382,9 +368,6 @@ private SkyValue computeInternal(SkyKey skyKey, Environment env)

// Remove action from state map in case it's there (won't be unless it discovers inputs).
stateMap.remove(action);
if (sketch != null && actionLookupData.valueIsShareable()) {
topDownActionCache.put(sketch, result);
}
return result;
}

Expand Down

0 comments on commit 16753d6

Please sign in to comment.