Skip to content

Commit

Permalink
remote: --experimental_remote_fetch_outputs=minimal
Browse files Browse the repository at this point in the history
See bazelbuild#6862 for details.
  • Loading branch information
buchgr committed Dec 9, 2018
1 parent ba6fdb9 commit 471b8b7
Show file tree
Hide file tree
Showing 51 changed files with 1,039 additions and 296 deletions.
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ filegroup(
"//src/main/java/com/google/devtools/build/lib/remote/blobstore:srcs",
"//src/main/java/com/google/devtools/build/lib/remote/blobstore/http:srcs",
"//src/main/java/com/google/devtools/build/lib/remote/logging:srcs",
"//src/main/java/com/google/devtools/build/lib/remote/options:srcs",
"//src/main/java/com/google/devtools/build/lib/remote/util:srcs",
"//src/main/java/com/google/devtools/build/lib/rules/apple/cpp:srcs",
"//src/main/java/com/google/devtools/build/lib/rules/apple:srcs",
Expand Down Expand Up @@ -659,6 +660,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/graph",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/profiler/memory:current_rule_tracker",
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/rules/cpp:cpp_interface",
"//src/main/java/com/google/devtools/build/lib/shell",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.google.devtools.common.options.OptionsProvider;
import java.io.Closeable;
import java.io.IOException;
import java.util.Collection;
import java.util.Map;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -73,6 +74,8 @@ public enum ShowSubcommands {

private final ArtifactPathResolver pathResolver;

private final ImmutableList<Artifact> requiredLocalOutputs;

private ActionExecutionContext(
Executor executor,
MetadataProvider actionInputFileCache,
Expand All @@ -82,6 +85,7 @@ private ActionExecutionContext(
FileOutErr fileOutErr,
Map<String, String> clientEnv,
ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
ImmutableList<Artifact> requiredLocalOutputs,
@Nullable ArtifactExpander artifactExpander,
@Nullable SkyFunction.Environment env,
@Nullable FileSystem actionFileSystem,
Expand All @@ -101,6 +105,7 @@ private ActionExecutionContext(
this.pathResolver = ArtifactPathResolver.createPathResolver(actionFileSystem,
// executor is only ever null in testing.
executor == null ? null : executor.getExecRoot());
this.requiredLocalOutputs = Preconditions.checkNotNull(requiredLocalOutputs);
}

public ActionExecutionContext(
Expand All @@ -112,6 +117,7 @@ public ActionExecutionContext(
FileOutErr fileOutErr,
Map<String, String> clientEnv,
ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> topLevelFilesets,
ImmutableList<Artifact> requiredLocalOutputs,
ArtifactExpander artifactExpander,
@Nullable FileSystem actionFileSystem,
@Nullable Object skyframeDepsResult) {
Expand All @@ -124,6 +130,7 @@ public ActionExecutionContext(
fileOutErr,
clientEnv,
topLevelFilesets,
requiredLocalOutputs,
artifactExpander,
/*env=*/ null,
actionFileSystem,
Expand All @@ -149,6 +156,7 @@ public static ActionExecutionContext forInputDiscovery(
fileOutErr,
clientEnv,
ImmutableMap.of(),
/*requiredLocalOutputs=*/ ImmutableList.of(),
/*artifactExpander=*/ null,
env,
actionFileSystem,
Expand Down Expand Up @@ -306,6 +314,18 @@ public ActionKeyContext getActionKeyContext() {
return actionKeyContext;
}

/**
* Returns the collection of files that this command must write and make available via
* the local {@link com.google.devtools.build.lib.vfs.FileSystem}. The returned output
* artifacts are a subset of the action's output artifacts.
*
* <p>This is for use with remote execution, where as an optimization we don't want to
* download all output files.
*/
public Collection<Artifact> getRequiredLocalOutputs() {
return requiredLocalOutputs;
}

@Override
public void close() throws IOException {
fileOutErr.close();
Expand All @@ -325,6 +345,28 @@ public ActionExecutionContext withFileOutErr(FileOutErr fileOutErr) {
fileOutErr,
clientEnv,
topLevelFilesets,
requiredLocalOutputs,
artifactExpander,
env,
actionFileSystem,
skyframeDepsResult);
}

/**
* Allows us to create a new context that provides a list of output artifacts that need to
* be staged on the local filesystem.
*/
public ActionExecutionContext withRequiredLocalOutputs(Collection<? extends Artifact> requiredLocalOutputs) {
return new ActionExecutionContext(
executor,
actionInputFileCache,
actionInputPrefetcher,
actionKeyContext,
metadataHandler,
fileOutErr,
clientEnv,
topLevelFilesets,
ImmutableList.copyOf(requiredLocalOutputs),
artifactExpander,
env,
actionFileSystem,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;

import java.io.IOException;

/** Prefetches files to local disk. */
public interface ActionInputPrefetcher {
public static final ActionInputPrefetcher NONE =
new ActionInputPrefetcher() {
@Override
public void prefetchFiles(Iterable<? extends ActionInput> input) {
public void prefetchFiles(Iterable<? extends ActionInput> inputs,
MetadataProvider metadataProvider) {
// Do nothing.
}
};
Expand All @@ -28,5 +31,5 @@ public void prefetchFiles(Iterable<? extends ActionInput> input) {
*
* <p>For any path not under this prefetcher's control, the call should be a no-op.
*/
void prefetchFiles(Iterable<? extends ActionInput> input);
void prefetchFiles(Iterable<? extends ActionInput> inputs, MetadataProvider metadataProvider) throws IOException, InterruptedException;
}
14 changes: 14 additions & 0 deletions src/main/java/com/google/devtools/build/lib/actions/Actions.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.vfs.OsPathPolicy;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
Expand Down Expand Up @@ -369,4 +371,16 @@ public ImmutableList<ActionAnalysisMetadata> getActions() {
return actions;
}
}

public static void prefetchInputs(Iterable<? extends ActionInput> inputs,
ActionExecutionContext actionExecutionContext, Action action) throws ActionExecutionException,
InterruptedException {
try {
actionExecutionContext
.getActionInputPrefetcher()
.prefetchFiles(inputs, actionExecutionContext.getMetadataProvider());
} catch (IOException e) {
throw new ActionExecutionException("Failed to (pre)fetch remote input files.", e, action, false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ public boolean isSourceRoot() {
return rootType == RootType.Source;
}

public boolean isOutputRoot() {
return rootType == RootType.Output;
}

boolean isMiddlemanRoot() {
return rootType == RootType.Middleman;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
* <li>a "middleman marker" object, which has a null digest, 0 size, and mtime of 0.
* <li>The "self data" of a TreeArtifact, where we would expect to see a digest representing the
* artifact's contents, and a size of 0.
* <li>a file that's only stored by a remote caching/execution system, in which case we would
* expect to see a digest and size.
* </ul>
*/
@Immutable
Expand Down Expand Up @@ -125,6 +127,13 @@ public final boolean isMarkerValue() {
*/
public abstract boolean wasModifiedSinceDigest(Path path) throws IOException;

/**
* Returns {@code true} if the file only exists remotely.
*/
public boolean isRemote() {
return false;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down Expand Up @@ -471,7 +480,12 @@ public int getLocationIndex() {

@Override
public boolean wasModifiedSinceDigest(Path path) {
throw new UnsupportedOperationException();
return false;
}

@Override
public boolean isRemote() {
return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* <p>Note that implementations of this interface call chmod on output files if {@link
* #discardOutputMetadata} has been called.
*/
public interface MetadataHandler extends MetadataProvider {
public interface MetadataHandler extends MetadataProvider, MetadataInjector {
@Override
FileArtifactValue getMetadata(ActionInput actionInput) throws IOException;

Expand All @@ -56,9 +56,6 @@ public interface MetadataHandler extends MetadataProvider {
*/
void injectDigest(ActionInput output, FileStatus statNoFollow, byte[] digest);

/** Injects a file that is only stored remotely. */
void injectRemoteFile(Artifact output, byte[] digest, long size, int locationIndex);

/**
* Marks an artifact as intentionally omitted. Acknowledges that this Artifact could have existed,
* but was intentionally not saved, most likely as an optimization.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2018 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.actions.cache;

import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Map;

/**
* Allows to inject metadata of action outputs into skyframe.
*/
public interface MetadataInjector {

/**
* Injects metadata of a file that is stored remotely.
*
* @param file a regular output file.
* @param digest the digest of the file.
* @param sizeBytes the size of the file in bytes.
* @param locationIndex is only used in Blaze.
*/
default void injectRemoteFile(Artifact file, byte[] digest, long sizeBytes, int locationIndex) {
throw new UnsupportedOperationException();
}

/**
* Inject the metadata of a tree artifact whose contents are stored remotely.
*
* @param treeArtifact an output directory.
* @param children the metadata of the files stored in the directory.
*/
default void injectRemoteDirectory(SpecialArtifact treeArtifact,
Map<PathFragment, RemoteFileArtifactValue> children) {
throw new UnsupportedOperationException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.protobuf.ByteString;
import java.io.IOException;
import java.io.OutputStream;
import java.util.Collections;

/**
* Abstract Action to write to a file.
Expand Down Expand Up @@ -59,6 +61,15 @@ public boolean makeExecutable() {
@Override
public final ActionResult execute(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
Actions.prefetchInputs(getInputs(), actionExecutionContext, this);
try {
actionExecutionContext
.getActionInputPrefetcher()
.prefetchFiles(getInputs(), actionExecutionContext.getMetadataProvider());
} catch (IOException e) {
throw new ActionExecutionException("Failed to fetch remote input file.", e, this, false);
}

ActionResult actionResult;
try {
DeterministicWriter deterministicWriter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import javax.annotation.Nullable;

/** Action to create a symbolic link. */
Expand Down Expand Up @@ -162,7 +165,9 @@ public Path getOutputPath(ActionExecutionContext actionExecutionContext) {

@Override
public ActionResult execute(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException {
throws ActionExecutionException, InterruptedException {
Actions.prefetchInputs(Collections.singletonList(getPrimaryInput()), actionExecutionContext,
this);
maybeVerifyTargetIsExecutable(actionExecutionContext);

Path srcPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ExecException;
Expand All @@ -34,6 +35,7 @@
import com.google.devtools.build.lib.util.Fingerprint;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.List;

/** Action to expand a template and write the expanded content to a file. */
Expand Down Expand Up @@ -127,6 +129,7 @@ public String getSkylarkContent() throws IOException {
@Override
public final ActionResult execute(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
Actions.prefetchInputs(getInputs(), actionExecutionContext, this);
TemplateExpansionContext expansionContext =
actionExecutionContext.getContext(TemplateExpansionContext.class);
try {
Expand Down
Loading

0 comments on commit 471b8b7

Please sign in to comment.