Let `SkyKey` alone declare its value's shareability.
There are currently three methods for declaring shareability:
* `SkyFunctionName#getShareabilityOfValue`
* `SkyKey#getShareabilityOfValue`
* `SkyValue#dataIsShareable`
The former two are just "hints" - a return of `SOMETIMES` still requires checking the value. However, there is no enforcement of consistency - for example, we can have a particular `SkyFunctionName#getShareabilityOfValue` return `NEVER`, but that function may compute a value whose `SkyValue#dataIsShareable` returns `true`. This currently happens in practice.
My original plan was to check consistency in `SerializationCheckingGraph`, but it turns out that it's not too difficult to just make `SkyKey` the sole proprietor of shareability. This is strictly better than giving the responsibility to `SkyValue` because a remote storage fetch for a value need not be attempted if the key is not shareable (this is what the "hint" in `SkyKey` intended to achieve).
Replace `ShareabilityOfValue` with a simple boolean since the return of `SkyKey#valueIsShareable` is now definite.
PiperOrigin-RevId: 416937942
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
index 7e4c87e..4e4282c 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
@@ -18,7 +18,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.skyframe.ExecutionPhaseSkyKey;
-import com.google.devtools.build.skyframe.ShareabilityOfValue;
import com.google.devtools.build.skyframe.SkyFunctionName;
/** Data that uniquely identifies an action. */
@@ -44,7 +43,10 @@
: new ActionLookupData(actionLookupKey, actionIndex);
}
- /** Similar to {@link #create}, but the key will have {@link ShareabilityOfValue#NEVER}. */
+ /**
+ * Similar to {@link #create}, but the key will return {@code false} for {@link
+ * #valueIsShareable}.
+ */
public static ActionLookupData createUnshareable(
ActionLookupKey actionLookupKey, int actionIndex) {
return new UnshareableActionLookupData(actionLookupKey, actionIndex);
@@ -58,21 +60,25 @@
* Index of the action in question in the node keyed by {@link #getActionLookupKey}. Should be
* passed to {@link ActionLookupValue#getAction}.
*/
- public int getActionIndex() {
+ public final int getActionIndex() {
return actionIndex;
}
- public Label getLabel() {
+ public final Label getLabel() {
return actionLookupKey.getLabel();
}
@Override
- public int hashCode() {
- return 37 * actionLookupKey.hashCode() + actionIndex;
+ public final int hashCode() {
+ int hash = 1;
+ hash = 37 * hash + actionLookupKey.hashCode();
+ hash = 37 * hash + Integer.hashCode(actionIndex);
+ hash = 37 * hash + Boolean.hashCode(valueIsShareable());
+ return hash;
}
@Override
- public boolean equals(Object obj) {
+ public final boolean equals(Object obj) {
if (this == obj) {
return true;
}
@@ -81,7 +87,8 @@
}
ActionLookupData that = (ActionLookupData) obj;
return this.actionIndex == that.actionIndex
- && this.actionLookupKey.equals(that.actionLookupKey);
+ && this.actionLookupKey.equals(that.actionLookupKey)
+ && valueIsShareable() == that.valueIsShareable();
}
@Override
@@ -103,8 +110,8 @@
}
@Override
- public ShareabilityOfValue getShareabilityOfValue() {
- return ShareabilityOfValue.NEVER;
+ public boolean valueIsShareable() {
+ return false;
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index 1213077..6c2ebc8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -49,7 +49,6 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
-import com.google.devtools.build.skyframe.ShareabilityOfValue;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.protobuf.CodedInputStream;
@@ -1117,8 +1116,8 @@
}
@Override
- public ShareabilityOfValue getShareabilityOfValue() {
- return isConstantMetadata() ? ShareabilityOfValue.NEVER : super.getShareabilityOfValue();
+ public boolean valueIsShareable() {
+ return !isConstantMetadata();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
index 5800986..d74ed51 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
@@ -201,48 +201,34 @@
isFile,
isFile ? fileValue.getSize() : 0,
isFile ? fileValue.realFileStateValue().getContentsProxy() : null,
- isFile ? fileValue.getDigest() : null,
- /* isShareable=*/ true);
+ isFile ? fileValue.getDigest() : null);
}
public static FileArtifactValue createFromInjectedDigest(
- FileArtifactValue metadata, @Nullable byte[] digest, boolean isShareable) {
- return createForNormalFile(
- digest, metadata.getContentsProxy(), metadata.getSize(), isShareable);
+ FileArtifactValue metadata, @Nullable byte[] digest) {
+ return createForNormalFile(digest, metadata.getContentsProxy(), metadata.getSize());
}
@VisibleForTesting
public static FileArtifactValue createForTesting(Artifact artifact) throws IOException {
- Path path = artifact.getPath();
- boolean isShareable = !artifact.isConstantMetadata();
- // Caution: there's a race condition between stating the file and computing the
- // digest. We need to stat first, since we're using the stat to detect changes.
- // We follow symlinks here to be consistent with getDigest.
- return createFromStat(path, path.stat(Symlinks.FOLLOW), isShareable);
+ return createForTesting(artifact.getPath());
}
@VisibleForTesting
public static FileArtifactValue createForTesting(Path path) throws IOException {
- /*isShareable=*/
- // Caution: there's a race condition between stating the file and computing the
- // digest. We need to stat first, since we're using the stat to detect changes.
- // We follow symlinks here to be consistent with getDigest.
- return createFromStat(path, path.stat(Symlinks.FOLLOW), true);
+ // Caution: there's a race condition between stating the file and computing the digest. We need
+ // to stat first, since we're using the stat to detect changes. We follow symlinks here to be
+ // consistent with getDigest.
+ return createFromStat(path, path.stat(Symlinks.FOLLOW));
}
- public static FileArtifactValue createFromStat(Path path, FileStatus stat, boolean isShareable)
- throws IOException {
+ public static FileArtifactValue createFromStat(Path path, FileStatus stat) throws IOException {
return create(
- path, stat.isFile(), stat.getSize(), FileContentsProxy.create(stat), null, isShareable);
+ path, stat.isFile(), stat.getSize(), FileContentsProxy.create(stat), /*digest=*/ null);
}
private static FileArtifactValue create(
- Path path,
- boolean isFile,
- long size,
- FileContentsProxy proxy,
- @Nullable byte[] digest,
- boolean isShareable)
+ Path path, boolean isFile, long size, FileContentsProxy proxy, @Nullable byte[] digest)
throws IOException {
if (!isFile) {
// In this case, we need to store the mtime because the action cache uses mtime for
@@ -254,7 +240,7 @@
digest = DigestUtils.getDigestWithManualFallback(path, size);
}
Preconditions.checkState(digest != null, path);
- return createForNormalFile(digest, proxy, size, isShareable);
+ return createForNormalFile(digest, proxy, size);
}
public static FileArtifactValue createForVirtualActionInput(byte[] digest, long size) {
@@ -281,10 +267,8 @@
@VisibleForTesting
public static FileArtifactValue createForNormalFile(
- byte[] digest, @Nullable FileContentsProxy proxy, long size, boolean isShareable) {
- return isShareable
- ? new RegularFileArtifactValue(digest, proxy, size)
- : new UnshareableRegularFileArtifactValue(digest, proxy, size);
+ byte[] digest, @Nullable FileContentsProxy proxy, long size) {
+ return new RegularFileArtifactValue(digest, proxy, size);
}
/**
@@ -293,7 +277,7 @@
*/
public static FileArtifactValue createForNormalFileUsingPath(Path path, long size)
throws IOException {
- return create(path, true, size, null, null, true);
+ return create(path, /*isFile=*/ true, size, /*proxy=*/ null, /*digest=*/ null);
}
public static FileArtifactValue createForDirectoryWithHash(byte[] digest) {
@@ -310,7 +294,7 @@
*/
public static FileArtifactValue createProxy(byte[] digest) {
Preconditions.checkNotNull(digest);
- return createForNormalFile(digest, /*proxy=*/ null, /*size=*/ 0, /*isShareable=*/ true);
+ return createForNormalFile(digest, /*proxy=*/ null, /*size=*/ 0);
}
private static String bytesToString(byte[] bytes) {
@@ -447,8 +431,7 @@
}
}
- private static class RegularFileArtifactValue extends FileArtifactValue {
-
+ private static final class RegularFileArtifactValue extends FileArtifactValue {
private final byte[] digest;
@Nullable private final FileContentsProxy proxy;
private final long size;
@@ -462,20 +445,21 @@
@Override
public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
if (!(o instanceof RegularFileArtifactValue)) {
return false;
}
-
RegularFileArtifactValue that = (RegularFileArtifactValue) o;
return Arrays.equals(digest, that.digest)
&& Objects.equals(proxy, that.proxy)
- && size == that.size
- && dataIsShareable() == that.dataIsShareable();
+ && size == that.size;
}
@Override
public int hashCode() {
- return Objects.hash(Arrays.hashCode(digest), proxy, size, dataIsShareable());
+ return Objects.hash(Arrays.hashCode(digest), proxy, size);
}
@Override
@@ -535,18 +519,6 @@
}
}
- private static final class UnshareableRegularFileArtifactValue extends RegularFileArtifactValue {
- private UnshareableRegularFileArtifactValue(
- byte[] digest, @Nullable FileContentsProxy proxy, long size) {
- super(digest, proxy, size);
- }
-
- @Override
- public boolean dataIsShareable() {
- return false;
- }
- }
-
/** Metadata for remotely stored files. */
public static class RemoteFileArtifactValue extends FileArtifactValue {
private final byte[] digest;
@@ -575,14 +547,12 @@
return Arrays.equals(digest, that.digest)
&& size == that.size
&& locationIndex == that.locationIndex
- && Objects.equals(actionId, that.actionId)
- && dataIsShareable() == that.dataIsShareable();
+ && Objects.equals(actionId, that.actionId);
}
@Override
public int hashCode() {
- return Objects.hash(
- Arrays.hashCode(digest), size, locationIndex, actionId, dataIsShareable());
+ return Objects.hash(Arrays.hashCode(digest), size, locationIndex, actionId);
}
@Override
@@ -682,7 +652,12 @@
}
/** File stored inline in metadata. */
- public static class InlineFileArtifactValue extends FileArtifactValue {
+ public static final class InlineFileArtifactValue extends FileArtifactValue {
+
+ public static InlineFileArtifactValue create(byte[] bytes, HashFunction hashFunction) {
+ return new InlineFileArtifactValue(bytes, hashFunction.hashBytes(bytes).asBytes());
+ }
+
private final byte[] data;
private final byte[] digest;
@@ -691,30 +666,21 @@
this.digest = Preconditions.checkNotNull(digest);
}
- private InlineFileArtifactValue(byte[] bytes, HashFunction hashFunction) {
- this(bytes, hashFunction.hashBytes(bytes).asBytes());
- }
-
@Override
public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
if (!(o instanceof InlineFileArtifactValue)) {
return false;
}
-
InlineFileArtifactValue that = (InlineFileArtifactValue) o;
- return Arrays.equals(digest, that.digest) && dataIsShareable() == that.dataIsShareable();
+ return Arrays.equals(digest, that.digest);
}
@Override
public int hashCode() {
- return Objects.hash(Arrays.hashCode(digest), dataIsShareable());
- }
-
- public static InlineFileArtifactValue create(
- byte[] bytes, boolean shareable, HashFunction hashFunction) {
- return shareable
- ? new InlineFileArtifactValue(bytes, hashFunction)
- : new UnshareableInlineFileArtifactValue(bytes, hashFunction);
+ return Arrays.hashCode(digest);
}
public ByteArrayInputStream getInputStream() {
@@ -752,17 +718,6 @@
}
}
- private static final class UnshareableInlineFileArtifactValue extends InlineFileArtifactValue {
- UnshareableInlineFileArtifactValue(byte[] bytes, HashFunction hashFunction) {
- super(bytes, hashFunction);
- }
-
- @Override
- public boolean dataIsShareable() {
- return false;
- }
- }
-
/**
* Used to resolve source symlinks when diskless.
*
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
index 7588e81..1a0ff62 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
@@ -60,7 +60,7 @@
Path path = ActionInputHelper.toInputPath(input, execRoot);
FileArtifactValue metadata;
try {
- metadata = FileArtifactValue.createFromStat(path, path.stat(Symlinks.FOLLOW), true);
+ metadata = FileArtifactValue.createFromStat(path, path.stat(Symlinks.FOLLOW));
} catch (IOException e) {
return new ActionInputMetadata(input, e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionFileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionFileArtifactValue.java
index e3e96f6..e3e30f7 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionFileArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionFileArtifactValue.java
@@ -46,18 +46,12 @@
&& getSize() == that.getSize()
&& getLocationIndex() == that.getLocationIndex()
&& Objects.equals(getActionId(), that.getActionId())
- && isExecutable == that.isExecutable
- && dataIsShareable() == that.dataIsShareable();
+ && isExecutable == that.isExecutable;
}
@Override
public int hashCode() {
return Objects.hash(
- Arrays.hashCode(getDigest()),
- getSize(),
- getLocationIndex(),
- getActionId(),
- isExecutable,
- dataIsShareable());
+ Arrays.hashCode(getDigest()), getSize(), getLocationIndex(), getActionId(), isExecutable);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 1c1b7ee..60d1d48 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -382,7 +382,7 @@
// Remove action from state map in case it's there (won't be unless it discovers inputs).
stateMap.remove(action);
- if (sketch != null && result.dataIsShareable()) {
+ if (sketch != null && actionLookupData.valueIsShareable()) {
topDownActionCache.put(sketch, result);
}
return result;
@@ -819,7 +819,7 @@
+ "SkyframeAwareAction which should be re-executed unconditionally. Action: %s",
action);
return ActionExecutionValue.createFromOutputStore(
- metadataHandler.getOutputStore(), /*outputSymlinks=*/ null, action, actionLookupData);
+ metadataHandler.getOutputStore(), /*outputSymlinks=*/ null, action);
}
metadataHandler.prepareForActionExecution();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
index a17fef5..65ee2fa 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
@@ -21,8 +21,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact;
import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
@@ -36,7 +34,6 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.rules.cpp.IncludeScannable;
import com.google.devtools.build.lib.skyframe.TreeArtifactValue.ArchivedRepresentation;
-import com.google.devtools.build.skyframe.ShareabilityOfValue;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Map;
import java.util.NoSuchElementException;
@@ -46,7 +43,7 @@
/** A value representing an executed action. */
@Immutable
@ThreadSafe
-public class ActionExecutionValue implements SkyValue {
+public final class ActionExecutionValue implements SkyValue {
/** A map from each output artifact of this action to their {@link FileArtifactValue}s. */
private final ImmutableMap<Artifact, FileArtifactValue> artifactData;
@@ -107,31 +104,23 @@
static ActionExecutionValue createFromOutputStore(
OutputStore outputStore,
@Nullable ImmutableList<FilesetOutputSymlink> outputSymlinks,
- Action action,
- ActionLookupData lookupData) {
- return create(
+ Action action) {
+ return new ActionExecutionValue(
outputStore.getAllArtifactData(),
outputStore.getAllTreeArtifactData(),
outputSymlinks,
action instanceof IncludeScannable
? ((IncludeScannable) action).getDiscoveredModules()
- : null,
- !Actions.dependsOnBuildId(action)
- && lookupData.getShareabilityOfValue() != ShareabilityOfValue.NEVER);
+ : null);
}
@VisibleForTesting
- static ActionExecutionValue create(
+ public static ActionExecutionValue createForTesting(
ImmutableMap<Artifact, FileArtifactValue> artifactData,
ImmutableMap<Artifact, TreeArtifactValue> treeArtifactData,
- @Nullable ImmutableList<FilesetOutputSymlink> outputSymlinks,
- @Nullable NestedSet<Artifact> discoveredModules,
- boolean shareable) {
- return shareable
- ? new ActionExecutionValue(
- artifactData, treeArtifactData, outputSymlinks, discoveredModules)
- : new CrossServerUnshareableActionExecutionValue(
- artifactData, treeArtifactData, outputSymlinks, discoveredModules);
+ @Nullable ImmutableList<FilesetOutputSymlink> outputSymlinks) {
+ return new ActionExecutionValue(
+ artifactData, treeArtifactData, outputSymlinks, /*discoveredModules=*/ null);
}
/**
@@ -230,33 +219,12 @@
ActionExecutionValue o = (ActionExecutionValue) obj;
return artifactData.equals(o.artifactData)
&& treeArtifactData.equals(o.treeArtifactData)
- && dataIsShareable() == o.dataIsShareable()
&& Objects.equal(outputSymlinks, o.outputSymlinks);
}
@Override
public int hashCode() {
- return Objects.hashCode(artifactData, treeArtifactData);
- }
-
- /**
- * Subclass that reports this value cannot be shared across servers. Note that this is unrelated
- * to the concept of shared actions.
- */
- private static final class CrossServerUnshareableActionExecutionValue
- extends ActionExecutionValue {
- CrossServerUnshareableActionExecutionValue(
- ImmutableMap<Artifact, FileArtifactValue> artifactData,
- ImmutableMap<Artifact, TreeArtifactValue> treeArtifactData,
- @Nullable ImmutableList<FilesetOutputSymlink> outputSymlinks,
- @Nullable NestedSet<Artifact> discoveredModules) {
- super(artifactData, treeArtifactData, outputSymlinks, discoveredModules);
- }
-
- @Override
- public boolean dataIsShareable() {
- return false;
- }
+ return Objects.hashCode(artifactData, treeArtifactData, outputSymlinks);
}
private static <V> ImmutableMap<Artifact, V> transformMap(
@@ -327,13 +295,12 @@
action);
Map<OwnerlessArtifactWrapper, Artifact> newArtifactMap =
Maps.uniqueIndex(action.getOutputs(), OwnerlessArtifactWrapper::new);
- return create(
+ return new ActionExecutionValue(
transformMap(artifactData, newArtifactMap, action, (newArtifact, value) -> value),
transformMap(
treeArtifactData, newArtifactMap, action, ActionExecutionValue::transformSharedTree),
outputSymlinks,
// Discovered modules come from the action's inputs, and so don't need to be transformed.
- discoveredModules,
- dataIsShareable());
+ discoveredModules);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index 52464a9..3171f12 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -556,8 +556,7 @@
injectedDigest =
DigestUtils.manuallyComputeDigest(artifactPathResolver.toPath(artifact), value.getSize());
}
- return FileArtifactValue.createFromInjectedDigest(
- value, injectedDigest, /*isShareable=*/ !artifact.isConstantMetadata());
+ return FileArtifactValue.createFromInjectedDigest(value, injectedDigest);
}
/**
@@ -608,7 +607,6 @@
rootedPathNoFollow,
statNoFollow,
digestWillBeInjected,
- artifact.isConstantMetadata(),
tsgm);
}
@@ -637,7 +635,6 @@
realRootedPath,
realStatWithDigest,
digestWillBeInjected,
- artifact.isConstantMetadata(),
tsgm);
}
@@ -645,7 +642,6 @@
RootedPath rootedPath,
FileStatusWithDigest stat,
boolean digestWillBeInjected,
- boolean isConstantMetadata,
@Nullable TimestampGranularityMonitor tsgm)
throws IOException {
if (stat == null) {
@@ -658,10 +654,7 @@
return stat.isDirectory()
? FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime())
: FileArtifactValue.createForNormalFile(
- fileStateValue.getDigest(),
- fileStateValue.getContentsProxy(),
- stat.getSize(),
- /*isShareable=*/ !isConstantMetadata);
+ fileStateValue.getDigest(), fileStateValue.getContentsProxy(), stat.getSize());
}
private static void setPathReadOnlyAndExecutableIfFile(Path path) throws IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java
index f535e92..c9f6fe5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetKey.java
@@ -17,7 +17,6 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.skyframe.ExecutionPhaseSkyKey;
-import com.google.devtools.build.skyframe.ShareabilityOfValue;
import com.google.devtools.build.skyframe.SkyFunctionName;
/** SkyKey for {@code NestedSet<Artifact>}. */
@@ -47,10 +46,10 @@
}
@Override
- public ShareabilityOfValue getShareabilityOfValue() {
+ public boolean valueIsShareable() {
// ArtifactNestedSetValue is just a promise that data is available in memory. Not meant for
// cross-server sharing.
- return ShareabilityOfValue.NEVER;
+ return false;
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetValue.java
index 7619343..0e78619 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetValue.java
@@ -23,11 +23,4 @@
*/
@Immutable
@ThreadSafe
-public final class ArtifactNestedSetValue implements SkyValue {
-
- @Override
- public boolean dataIsShareable() {
- // This is just a promise that data is available in memory. Not meant for cross-server sharing.
- return false;
- }
-}
+public final class ArtifactNestedSetValue implements SkyValue {}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
index 6eb9b44..2e56754 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
@@ -49,8 +49,13 @@
public abstract AspectKey actionLookupKey();
@Override
- public SkyFunctionName functionName() {
+ public final SkyFunctionName functionName() {
return SkyFunctions.ASPECT_COMPLETION;
}
+
+ @Override
+ public final boolean valueIsShareable() {
+ return false;
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadValue.java
index 83b8c8e..609af60 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadValue.java
@@ -103,6 +103,13 @@
public SkyFunctionName functionName() {
return SkyFunctions.BZL_LOAD;
}
+
+ @Override
+ public final boolean valueIsShareable() {
+ // We don't guarantee that all constructs implement equality, meaning we can't correctly
+ // compare deserialized instances. This is currently the case for attribute descriptors.
+ return false;
+ }
}
/** A key for loading a .bzl during package loading (BUILD evaluation). */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
index e1a64e3..6518feb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
@@ -23,10 +24,12 @@
import com.google.devtools.build.lib.packages.RuleVisibility;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
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.skyframe.AbstractSkyKey;
import com.google.devtools.build.skyframe.Injectable;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Map;
import java.util.UUID;
@@ -38,7 +41,7 @@
* "precomputed" from skyframe's perspective and so the graph needs to be prepopulated with them
* (e.g. via injection).
*/
-public class PrecomputedValue implements SkyValue {
+public final class PrecomputedValue implements SkyValue {
/**
* An externally-injected precomputed value. Exists so that modules can inject precomputed values
* into Skyframe's graph.
@@ -47,9 +50,9 @@
*/
public static final class Injected {
private final Precomputed<?> precomputed;
- private final Supplier<? extends Object> supplier;
+ private final Supplier<?> supplier;
- private Injected(Precomputed<?> precomputed, Supplier<? extends Object> supplier) {
+ private Injected(Precomputed<?> precomputed, Supplier<?> supplier) {
this.precomputed = precomputed;
this.supplier = supplier;
}
@@ -81,7 +84,7 @@
public static final Precomputed<StarlarkSemantics> STARLARK_SEMANTICS =
new Precomputed<>("starlark_semantics");
- static final Precomputed<UUID> BUILD_ID = new UnsharablePrecomputed<>("build_id");
+ static final Precomputed<UUID> BUILD_ID = new Precomputed<>("build_id", /*shareable=*/ false);
public static final Precomputed<Map<String, String>> ACTION_ENV = new Precomputed<>("action_env");
@@ -131,15 +134,19 @@
*
* <p>Instances do not have internal state.
*/
- public static class Precomputed<T> {
- protected final Key key;
+ public static final class Precomputed<T> {
+ private final SkyKey key;
public Precomputed(String key) {
- this.key = Key.create(key);
+ this(key, /*shareable=*/ true);
+ }
+
+ private Precomputed(String key, boolean shareable) {
+ this.key = shareable ? Key.create(key) : UnshareableKey.create(key);
}
@VisibleForTesting
- public Key getKeyForTesting() {
+ public SkyKey getKeyForTesting() {
return key;
}
@@ -162,35 +169,19 @@
public void set(Injectable injectable, T value) {
injectable.inject(key, new PrecomputedValue(value));
}
- }
-
- private static class UnsharablePrecomputed<T> extends Precomputed<T> {
- private UnsharablePrecomputed(String key) {
- super(key);
- }
-
- /** Injects a new variable value. */
- @Override
- public void set(Injectable injectable, T value) {
- injectable.inject(key, new UnshareablePrecomputedValue(value));
- }
- }
-
- /** An unshareable version of {@link PrecomputedValue}. */
- private static final class UnshareablePrecomputedValue extends PrecomputedValue {
- private UnshareablePrecomputedValue(Object value) {
- super(value);
- }
@Override
- public boolean dataIsShareable() {
- return false;
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("key", key)
+ .add("shareable", key.valueIsShareable())
+ .toString();
}
}
/** {@link com.google.devtools.build.skyframe.SkyKey} for {@code PrecomputedValue}. */
@AutoCodec
- public static class Key extends AbstractSkyKey<String> {
+ public static final class Key extends AbstractSkyKey<String> {
private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
private Key(String arg) {
@@ -207,4 +198,31 @@
return SkyFunctions.PRECOMPUTED;
}
}
+
+ /** Unshareable version of {@link Key}. */
+ @AutoCodec
+ @VisibleForSerialization
+ static final class UnshareableKey extends AbstractSkyKey<String> {
+ private static final Interner<UnshareableKey> interner = BlazeInterners.newWeakInterner();
+
+ private UnshareableKey(String arg) {
+ super(arg);
+ }
+
+ @AutoCodec.Instantiator
+ @VisibleForSerialization
+ static UnshareableKey create(String arg) {
+ return interner.intern(new UnshareableKey(arg));
+ }
+
+ @Override
+ public SkyFunctionName functionName() {
+ return SkyFunctions.PRECOMPUTED;
+ }
+
+ @Override
+ public boolean valueIsShareable() {
+ return false;
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index 9d8cd42..11c46c3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -13,10 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Predicate;
-import com.google.devtools.build.skyframe.FunctionHermeticity;
-import com.google.devtools.build.skyframe.ShareabilityOfValue;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
@@ -42,10 +39,7 @@
public static final SkyFunctionName BZL_COMPILE = SkyFunctionName.createHermetic("BZL_COMPILE");
public static final SkyFunctionName STARLARK_BUILTINS =
SkyFunctionName.createHermetic("STARLARK_BUILTINS");
- // Never shareable - we don't guarantee that all constructs implement equality, meaning we can't
- // correctly compare deserialized instances. This is currently the case for attribute descriptors.
- public static final SkyFunctionName BZL_LOAD =
- SkyFunctionName.create("BZL_LOAD", ShareabilityOfValue.NEVER, FunctionHermeticity.HERMETIC);
+ public static final SkyFunctionName BZL_LOAD = SkyFunctionName.createHermetic("BZL_LOAD");
public static final SkyFunctionName GLOB = SkyFunctionName.createHermetic("GLOB");
public static final SkyFunctionName PACKAGE = SkyFunctionName.createHermetic("PACKAGE");
static final SkyFunctionName PACKAGE_ERROR = SkyFunctionName.createHermetic("PACKAGE_ERROR");
@@ -89,35 +83,26 @@
static final SkyFunctionName TOP_LEVEL_ACTION_LOOKUP_CONFLICT_FINDING =
SkyFunctionName.createHermetic("TOP_LEVEL_ACTION_LOOKUP_CONFLICT_DETECTION");
public static final SkyFunctionName ASPECT = SkyFunctionName.createHermetic("ASPECT");
- static final SkyFunctionName LOAD_STARLARK_ASPECT =
- SkyFunctionName.createHermetic("LOAD_STARLARK_ASPECT");
static final SkyFunctionName TOP_LEVEL_ASPECTS =
SkyFunctionName.createHermetic("TOP_LEVEL_ASPECTS");
static final SkyFunctionName BUILD_TOP_LEVEL_ASPECTS_DETAILS =
SkyFunctionName.createHermetic("BUILD_TOP_LEVEL_ASPECTS_DETAILS");
public static final SkyFunctionName TARGET_COMPLETION =
- SkyFunctionName.create(
- "TARGET_COMPLETION", ShareabilityOfValue.NEVER, FunctionHermeticity.HERMETIC);
+ SkyFunctionName.createHermetic("TARGET_COMPLETION");
public static final SkyFunctionName ASPECT_COMPLETION =
- SkyFunctionName.create(
- "ASPECT_COMPLETION", ShareabilityOfValue.NEVER, FunctionHermeticity.HERMETIC);
- static final SkyFunctionName TEST_COMPLETION =
- SkyFunctionName.create(
- "TEST_COMPLETION", ShareabilityOfValue.NEVER, FunctionHermeticity.HERMETIC);
+ SkyFunctionName.createHermetic("ASPECT_COMPLETION");
+ static final SkyFunctionName TEST_COMPLETION = SkyFunctionName.createHermetic("TEST_COMPLETION");
public static final SkyFunctionName BUILD_CONFIGURATION =
SkyFunctionName.createHermetic("BUILD_CONFIGURATION");
- // Test actions are not shareable. Action execution can be nondeterministic, so is semi-hermetic.
+ // Action execution can be nondeterministic, so semi-hermetic.
public static final SkyFunctionName ACTION_EXECUTION =
SkyFunctionName.createSemiHermetic("ACTION_EXECUTION");
public static final SkyFunctionName ARTIFACT_NESTED_SET =
SkyFunctionName.createHermetic("ARTIFACT_NESTED_SET");
public static final SkyFunctionName PATH_CASING_LOOKUP =
SkyFunctionName.createHermetic("PATH_CASING_LOOKUP");
-
- @VisibleForTesting
- public static final SkyFunctionName RECURSIVE_FILESYSTEM_TRAVERSAL =
+ static final SkyFunctionName RECURSIVE_FILESYSTEM_TRAVERSAL =
SkyFunctionName.createHermetic("RECURSIVE_DIRECTORY_TRAVERSAL");
-
public static final SkyFunctionName FILESET_ENTRY =
SkyFunctionName.createHermetic("FILESET_ENTRY");
static final SkyFunctionName BUILD_INFO_COLLECTION =
@@ -148,14 +133,10 @@
SkyFunctionName.createHermetic("TOOLCHAIN_RESOLUTION");
public static final SkyFunctionName REPOSITORY_MAPPING =
SkyFunctionName.createHermetic("REPOSITORY_MAPPING");
- public static final SkyFunctionName REPO_MAPPING_FOR_BZLMOD_BZL_LOAD =
- SkyFunctionName.createHermetic("REPO_MAPPING_FOR_BZLMOD_BZL_LOAD");
public static final SkyFunctionName RESOLVED_FILE =
SkyFunctionName.createHermetic("RESOLVED_FILE");
public static final SkyFunctionName RESOLVED_HASH_VALUES =
SkyFunctionName.createHermetic("RESOLVED_HASH_VALUES");
- public static final SkyFunctionName LOCAL_CONFIG_PLATFORM =
- SkyFunctionName.createHermetic("LOCAL_CONFIG_PLATFORM");
public static final SkyFunctionName MODULE_FILE =
SkyFunctionName.createNonHermetic("MODULE_FILE");
public static final SkyFunctionName BUILD_DRIVER =
@@ -169,12 +150,7 @@
public static final SkyFunctionName SINGLE_EXTENSION_EVAL =
SkyFunctionName.createNonHermetic("SINGLE_EXTENSION_EVAL");
- public static Predicate<SkyKey> isSkyFunction(final SkyFunctionName functionName) {
- return new Predicate<SkyKey>() {
- @Override
- public boolean apply(SkyKey key) {
- return key.functionName().equals(functionName);
- }
- };
+ public static Predicate<SkyKey> isSkyFunction(SkyFunctionName functionName) {
+ return key -> key.functionName().equals(functionName);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index 3cfd0d8..3c90c17 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -1264,8 +1264,7 @@
return ActionExecutionValue.createFromOutputStore(
this.metadataHandler.getOutputStore(),
actionExecutionContext.getOutputSymlinks(),
- action,
- actionLookupData);
+ action);
}
/** A closure to continue an asynchronously running action. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
index ff34827..9ed1b2d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
@@ -68,10 +68,15 @@
public abstract ConfiguredTargetKey actionLookupKey();
@Override
- public SkyFunctionName functionName() {
+ public final SkyFunctionName functionName() {
return SkyFunctions.TARGET_COMPLETION;
}
+ @Override
+ public final boolean valueIsShareable() {
+ return false;
+ }
+
abstract boolean willTest();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java
index f55e507..cd2e12f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionValue.java
@@ -29,26 +29,22 @@
* A test completion value represents the completion of a test target. This includes the execution
* of all test shards and repeated runs, if applicable.
*/
-public class TestCompletionValue implements SkyValue {
+public final class TestCompletionValue implements SkyValue {
static final TestCompletionValue TEST_COMPLETION_MARKER = new TestCompletionValue();
private TestCompletionValue() { }
- @Override
- public boolean dataIsShareable() {
- return false;
- }
-
public static SkyKey key(
ConfiguredTargetKey lac,
- final TopLevelArtifactContext topLevelArtifactContext,
- final boolean exclusiveTesting) {
+ TopLevelArtifactContext topLevelArtifactContext,
+ boolean exclusiveTesting) {
return TestCompletionKey.create(lac, topLevelArtifactContext, exclusiveTesting);
}
- public static Iterable<SkyKey> keys(Collection<ConfiguredTarget> targets,
- final TopLevelArtifactContext topLevelArtifactContext,
- final boolean exclusiveTesting) {
+ public static Iterable<SkyKey> keys(
+ Collection<ConfiguredTarget> targets,
+ TopLevelArtifactContext topLevelArtifactContext,
+ boolean exclusiveTesting) {
return Iterables.transform(
targets,
ct ->
@@ -84,8 +80,13 @@
public abstract boolean exclusiveTesting();
@Override
- public SkyFunctionName functionName() {
+ public final SkyFunctionName functionName() {
return SkyFunctions.TEST_COMPLETION;
}
+
+ @Override
+ public final boolean valueIsShareable() {
+ return false;
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/BUILD b/src/main/java/com/google/devtools/build/skyframe/BUILD
index 7ab3fdd..91c12d5 100644
--- a/src/main/java/com/google/devtools/build/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/skyframe/BUILD
@@ -11,7 +11,6 @@
SKYFRAME_OBJECT_SRCS = [
"AbstractSkyKey.java",
"FunctionHermeticity.java",
- "ShareabilityOfValue.java",
"SkyFunctionName.java",
"SkyKey.java",
"SkyValue.java",
@@ -23,7 +22,7 @@
srcs = SKYFRAME_OBJECT_SRCS,
visibility = ["//visibility:public"],
deps = [
- "//third_party:caffeine",
+ "//src/main/java/com/google/devtools/build/lib/concurrent",
"//third_party:guava",
],
)
diff --git a/src/main/java/com/google/devtools/build/skyframe/ErrorTransienceValue.java b/src/main/java/com/google/devtools/build/skyframe/ErrorTransienceValue.java
index c3a84fb..941d03d 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ErrorTransienceValue.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ErrorTransienceValue.java
@@ -22,10 +22,26 @@
public final class ErrorTransienceValue implements SkyValue {
private static final SkyFunctionName FUNCTION_NAME =
- SkyFunctionName.create(
- "ERROR_TRANSIENCE", ShareabilityOfValue.NEVER, FunctionHermeticity.NONHERMETIC);
+ SkyFunctionName.createNonHermetic("ERROR_TRANSIENCE");
- @SerializationConstant public static final SkyKey KEY = () -> FUNCTION_NAME;
+ @SerializationConstant
+ public static final SkyKey KEY =
+ new SkyKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public boolean valueIsShareable() {
+ return false;
+ }
+
+ @Override
+ public String toString() {
+ return "ErrorTransienceValue.KEY";
+ }
+ };
@SerializationConstant
public static final ErrorTransienceValue INSTANCE = new ErrorTransienceValue();
@@ -33,11 +49,6 @@
private ErrorTransienceValue() {}
@Override
- public boolean dataIsShareable() {
- return false;
- }
-
- @Override
public int hashCode() {
// Not the prettiest, but since we always return false for equals throw exception here to catch
// any errors related to hash-based collections quickly.
diff --git a/src/main/java/com/google/devtools/build/skyframe/ShareabilityOfValue.java b/src/main/java/com/google/devtools/build/skyframe/ShareabilityOfValue.java
deleted file mode 100644
index 625c5db..0000000
--- a/src/main/java/com/google/devtools/build/skyframe/ShareabilityOfValue.java
+++ /dev/null
@@ -1,36 +0,0 @@
-// 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.skyframe;
-
-/**
- * When the {@link NodeEntry#getValue} corresponding to a given {@link SkyFunctionName} is
- * shareable: always, sometimes (depending on the specific key argument and/or value), or never.
- *
- * <p>Values may be unshareable because they are just not serializable, or because they contain data
- * that cannot safely be re-used as-is by another invocation.
- *
- * <p>Unshareable data should not be serialized, since it will never be re-used. Attempts to fetch
- * serialized data will check this value and only perform the fetch if the value is not {@link
- * #NEVER}.
- */
-public enum ShareabilityOfValue {
- /**
- * Indicates that values produced by the function are shareable unless they override {@link
- * SkyValue#dataIsShareable}.
- */
- SOMETIMES,
- /** Indicates that values produced by the function are not shareable. */
- NEVER
-}
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
index 623a8d2..3d3f356 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
@@ -13,23 +13,22 @@
// limitations under the License.
package com.google.devtools.build.skyframe;
-import com.github.benmanes.caffeine.cache.Cache;
-import com.github.benmanes.caffeine.cache.Caffeine;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
+import com.google.common.collect.Interner;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
import java.util.Set;
/** An identifier for a {@code SkyFunction}. */
public final class SkyFunctionName {
- private static final Cache<NameOnlyWrapper, SkyFunctionName> interner =
- Caffeine.newBuilder().build();
+ private static final Interner<SkyFunctionName> interner = BlazeInterners.newStrongInterner();
/**
* A well-known key type intended for testing only. The associated SkyKey should have a String
* argument.
*/
- // Needs to be after the cache is initialized.
+ // Needs to be after the interner is initialized.
public static final SkyFunctionName FOR_TESTING = SkyFunctionName.createHermetic("FOR_TESTING");
/**
@@ -39,7 +38,7 @@
* opposed to transitively invalidated).
*/
public static SkyFunctionName createNonHermetic(String name) {
- return create(name, ShareabilityOfValue.SOMETIMES, FunctionHermeticity.NONHERMETIC);
+ return create(name, FunctionHermeticity.NONHERMETIC);
}
/**
@@ -47,7 +46,7 @@
* FunctionHermeticity#SEMI_HERMETIC semi-hermetic}.
*/
public static SkyFunctionName createSemiHermetic(String name) {
- return create(name, ShareabilityOfValue.SOMETIMES, FunctionHermeticity.SEMI_HERMETIC);
+ return create(name, FunctionHermeticity.SEMI_HERMETIC);
}
/**
@@ -55,47 +54,40 @@
* to be a deterministic function of its dependencies, not doing any external operations).
*/
public static SkyFunctionName createHermetic(String name) {
- return create(name, ShareabilityOfValue.SOMETIMES, FunctionHermeticity.HERMETIC);
+ return create(name, FunctionHermeticity.HERMETIC);
}
- public static SkyFunctionName create(
- String name, ShareabilityOfValue shareabilityOfValue, FunctionHermeticity hermeticity) {
- SkyFunctionName result = new SkyFunctionName(name, shareabilityOfValue, hermeticity);
- SkyFunctionName cached = interner.get(new NameOnlyWrapper(result), unused -> result);
+ private static SkyFunctionName create(String name, FunctionHermeticity hermeticity) {
+ SkyFunctionName cached = interner.intern(new SkyFunctionName(name, hermeticity));
Preconditions.checkState(
- result.equals(cached),
- "Tried to create SkyFunctionName objects with same name but different properties: %s %s",
- result,
- cached);
+ cached.hermeticity.equals(hermeticity),
+ "Tried to create SkyFunctionName objects with same name (%s) but different hermeticity"
+ + " (old=%s, new=%s)",
+ name,
+ cached.hermeticity,
+ hermeticity);
return cached;
}
private final String name;
- private final ShareabilityOfValue shareabilityOfValue;
private final FunctionHermeticity hermeticity;
- private SkyFunctionName(
- String name, ShareabilityOfValue shareabilityOfValue, FunctionHermeticity hermeticity) {
- this.name = name;
- this.shareabilityOfValue = shareabilityOfValue;
- this.hermeticity = hermeticity;
+ private SkyFunctionName(String name, FunctionHermeticity hermeticity) {
+ this.name = Preconditions.checkNotNull(name);
+ this.hermeticity = Preconditions.checkNotNull(hermeticity);
}
public String getName() {
return name;
}
- public ShareabilityOfValue getShareabilityOfValue() {
- return shareabilityOfValue;
- }
-
public FunctionHermeticity getHermeticity() {
return hermeticity;
}
@Override
public String toString() {
- return name + (shareabilityOfValue.equals(ShareabilityOfValue.NEVER) ? " (unshareable)" : "");
+ return name;
}
@Override
@@ -107,48 +99,26 @@
return false;
}
SkyFunctionName other = (SkyFunctionName) obj;
- return name.equals(other.name) && shareabilityOfValue.equals(other.shareabilityOfValue);
+ return name.equals(other.name);
}
@Override
public int hashCode() {
- // Don't bother incorporating serializabilityOfValue into hashCode: should always be the same.
+ // Don't bother incorporating hermeticity into hashCode: should always be the same.
return name.hashCode();
}
/**
* A predicate that returns true for {@link SkyKey}s that have the given {@link SkyFunctionName}.
*/
- public static Predicate<SkyKey> functionIs(final SkyFunctionName functionName) {
+ public static Predicate<SkyKey> functionIs(SkyFunctionName functionName) {
return skyKey -> functionName.equals(skyKey.functionName());
}
/**
* A predicate that returns true for {@link SkyKey}s that have the given {@link SkyFunctionName}.
*/
- public static Predicate<SkyKey> functionIsIn(final Set<SkyFunctionName> functionNames) {
+ public static Predicate<SkyKey> functionIsIn(Set<SkyFunctionName> functionNames) {
return skyKey -> functionNames.contains(skyKey.functionName());
}
-
- private static class NameOnlyWrapper {
- private final SkyFunctionName skyFunctionName;
-
- private NameOnlyWrapper(SkyFunctionName skyFunctionName) {
- this.skyFunctionName = skyFunctionName;
- }
-
- @Override
- public boolean equals(Object obj) {
- if (!(obj instanceof NameOnlyWrapper)) {
- return false;
- }
- SkyFunctionName thatFunctionName = ((NameOnlyWrapper) obj).skyFunctionName;
- return this.skyFunctionName.getName().equals(thatFunctionName.name);
- }
-
- @Override
- public int hashCode() {
- return skyFunctionName.getName().hashCode();
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyKey.java b/src/main/java/com/google/devtools/build/skyframe/SkyKey.java
index 79284ac..f6cc4c0 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyKey.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyKey.java
@@ -33,7 +33,21 @@
return this;
}
- default ShareabilityOfValue getShareabilityOfValue() {
- return functionName().getShareabilityOfValue();
+ /**
+ * Returns {@code true} if this key produces a {@link SkyValue} that can be reused across builds.
+ *
+ * <p>Values may be unshareable because they are just not serializable, or because they contain
+ * data that cannot safely be reused as-is by another invocation, such as stamping information or
+ * "flaky" values like test statuses.
+ *
+ * <p>Unshareable data should not be serialized, since it will never be reused. Attempts to fetch
+ * a key's serialized data will call this method and only perform the fetch if it returns {@code
+ * true}.
+ *
+ * <p>The result of this method only applies to non-error values. In case of an error, {@link
+ * ErrorInfo#isTransitivelyTransient()} can be used to determine shareability.
+ */
+ default boolean valueIsShareable() {
+ return true;
}
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyValue.java b/src/main/java/com/google/devtools/build/skyframe/SkyValue.java
index 1c0e1c6..2d21e86 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyValue.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyValue.java
@@ -14,14 +14,4 @@
package com.google.devtools.build.skyframe;
/** A return value of a {@code SkyFunction}. */
-public interface SkyValue {
-
- /**
- * Returns true for values that can be reused across builds. Some values are inherently "flaky",
- * like test statuses or stamping information, and in certain circumstances, those values cannot
- * be shared across builds/servers.
- */
- default boolean dataIsShareable() {
- return true;
- }
-}
+public interface SkyValue {}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
index 07e50b7..77f8d20 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
@@ -96,8 +96,7 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact,
- FileArtifactValue.createForNormalFile(
- FAKE_DIGEST, /*proxy=*/ null, 0L, /*isShareable=*/ true));
+ FileArtifactValue.createForNormalFile(FAKE_DIGEST, /*proxy=*/ null, /*size=*/ 0L));
expander.addRunfilesToInputs(
inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT);
@@ -115,8 +114,7 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact,
- FileArtifactValue.createForNormalFile(
- FAKE_DIGEST, /*proxy=*/ null, 0L, /*isShareable=*/ true));
+ FileArtifactValue.createForNormalFile(FAKE_DIGEST, /*proxy=*/ null, /*size=*/ 0L));
ArtifactExpander filesetExpander =
new ArtifactExpander() {
@@ -206,12 +204,10 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact1,
- FileArtifactValue.createForNormalFile(
- FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true));
+ FileArtifactValue.createForNormalFile(FAKE_DIGEST, /*proxy=*/ null, /*size=*/ 1L));
mockCache.put(
artifact2,
- FileArtifactValue.createForNormalFile(
- FAKE_DIGEST, /*proxy=*/ null, 12L, /*isShareable=*/ true));
+ FileArtifactValue.createForNormalFile(FAKE_DIGEST, /*proxy=*/ null, /*size=*/ 12L));
expander.addRunfilesToInputs(
inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT);
@@ -237,8 +233,7 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact,
- FileArtifactValue.createForNormalFile(
- FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true));
+ FileArtifactValue.createForNormalFile(FAKE_DIGEST, /*proxy=*/ null, /*size=*/ 1L));
expander.addRunfilesToInputs(
inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT);
@@ -262,8 +257,7 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(
artifact,
- FileArtifactValue.createForNormalFile(
- FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true));
+ FileArtifactValue.createForNormalFile(FAKE_DIGEST, /*proxy=*/ null, /*size=*/ 1L));
expander.addRunfilesToInputs(
inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, PathFragment.EMPTY_FRAGMENT);
diff --git a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java
index 76dfc49..d29645c 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java
@@ -47,10 +47,7 @@
Path path = execRoot.getRelative(input.getExecPath());
FileStatus stat = path.stat(Symlinks.FOLLOW);
return FileArtifactValue.createForNormalFile(
- HashCode.fromString(hexDigest).asBytes(),
- FileContentsProxy.create(stat),
- stat.getSize(),
- /*isShareable=*/ true);
+ HashCode.fromString(hexDigest).asBytes(), FileContentsProxy.create(stat), stat.getSize());
}
@Override
@@ -58,13 +55,13 @@
throw new UnsupportedOperationException();
}
- void setDigest(ActionInput input, String digest) {
+ private void setDigest(ActionInput input, String digest) {
cas.put(input, digest);
}
public Digest createScratchInput(ActionInput input, String content) throws IOException {
Path inputFile = execRoot.getRelative(input.getExecPath());
- FileSystemUtils.createDirectoryAndParents(inputFile.getParentDirectory());
+ inputFile.getParentDirectory().createDirectoryAndParents();
FileSystemUtils.writeContentAsLatin1(inputFile, content);
Digest digest = digestUtil.compute(inputFile);
setDigest(input, digest.getHash());
@@ -73,7 +70,7 @@
public Digest createScratchInputDirectory(ActionInput input, Tree content) throws IOException {
Path inputFile = execRoot.getRelative(input.getExecPath());
- FileSystemUtils.createDirectoryAndParents(inputFile);
+ inputFile.createDirectoryAndParents();
Digest digest = digestUtil.compute(content);
setDigest(input, digest.getHash());
return digest;
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
index 85f324d..a34bb48 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -28,7 +29,6 @@
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
-import com.google.devtools.build.lib.clock.JavaClock;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -41,26 +41,21 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
/** Tests for {@link RemoteActionFileSystem} */
@RunWith(JUnit4.class)
-public class RemoteActionFileSystemTest {
+public final class RemoteActionFileSystemTest {
private static final DigestHashFunction HASH_FUNCTION = DigestHashFunction.SHA256;
- @Mock private RemoteActionInputFetcher inputFetcher;
- private FileSystem fs;
- private Path execRoot;
- private ArtifactRoot outputRoot;
+ private final RemoteActionInputFetcher inputFetcher = mock(RemoteActionInputFetcher.class);
+ private final FileSystem fs = new InMemoryFileSystem(HASH_FUNCTION);
+ private final Path execRoot = fs.getPath("/exec");
+ private final ArtifactRoot outputRoot =
+ ArtifactRoot.asDerivedRoot(execRoot, RootType.Output, "out");
@Before
- public void setUp() throws IOException {
- MockitoAnnotations.initMocks(this);
- fs = new InMemoryFileSystem(new JavaClock(), HASH_FUNCTION);
- execRoot = fs.getPath("/exec");
- outputRoot = ArtifactRoot.asDerivedRoot(execRoot, RootType.Output, "out");
+ public void createOutputRoot() throws IOException {
outputRoot.getRoot().asPath().createDirectoryAndParents();
}
@@ -188,8 +183,7 @@
// Caution: there's a race condition between stating the file and computing the
// digest. We need to stat first, since we're using the stat to detect changes.
// We follow symlinks here to be consistent with getDigest.
- inputs.putWithNoDepOwner(
- a, FileArtifactValue.createFromStat(path, path.stat(Symlinks.FOLLOW), true));
+ inputs.putWithNoDepOwner(a, FileArtifactValue.createFromStat(path, path.stat(Symlinks.FOLLOW)));
return a;
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java
index 938b8fb..0a31309 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java
@@ -256,12 +256,8 @@
private static ActionExecutionValue createActionExecutionValue(
ImmutableMap<Artifact, FileArtifactValue> fileArtifacts,
ImmutableMap<Artifact, TreeArtifactValue> treeArtifacts) {
- return ActionExecutionValue.create(
- fileArtifacts,
- treeArtifacts,
- /*outputSymlinks=*/ null,
- /*discoveredModules=*/ null,
- /*shareable=*/ true);
+ return ActionExecutionValue.createForTesting(
+ fileArtifacts, treeArtifacts, /*outputSymlinks=*/ null);
}
private static void createFile(Path file) throws IOException {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
index a2e46f3..3544479 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -106,8 +106,7 @@
public void withNonArtifactInput() throws Exception {
ActionInput input = ActionInputHelper.fromPath("foo/bar");
FileArtifactValue metadata =
- FileArtifactValue.createForNormalFile(
- new byte[] {1, 2, 3}, /*proxy=*/ null, 10L, /*isShareable=*/ true);
+ FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /*proxy=*/ null, /*size=*/ 10L);
ActionInputMap map = new ActionInputMap(1);
map.putWithNoDepOwner(input, metadata);
assertThat(map.getMetadata(input)).isEqualTo(metadata);
@@ -122,8 +121,7 @@
PathFragment path = PathFragment.create("src/a");
Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath(sourceRoot, path);
FileArtifactValue metadata =
- FileArtifactValue.createForNormalFile(
- new byte[] {1, 2, 3}, /*proxy=*/ null, 10L, /*isShareable=*/ true);
+ FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, /*proxy=*/ null, /*size=*/ 10L);
ActionInputMap map = new ActionInputMap(1);
map.putWithNoDepOwner(artifact, metadata);
ActionMetadataHandler handler =
@@ -411,10 +409,7 @@
// child is missing, getExistingFileArtifactValue will throw.
ActionExecutionValue actionExecutionValue =
ActionExecutionValue.createFromOutputStore(
- handler.getOutputStore(),
- /*outputSymlinks=*/ null,
- new NullAction(),
- ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ handler.getOutputStore(), /*outputSymlinks=*/ null, new NullAction());
tree.getChildren().forEach(actionExecutionValue::getExistingFileArtifactValue);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
index 37edcb0..a727496 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
@@ -87,23 +87,23 @@
delegateActionExecutionFunction = new SimpleActionExecutionFunction(omittedOutputs);
}
- private void assertFileArtifactValueMatches(boolean expectDigest) throws Throwable {
+ private void assertFileArtifactValueMatches() throws Exception {
Artifact output = createDerivedArtifact("output");
Path path = output.getPath();
file(path, "contents");
- assertValueMatches(path.stat(), expectDigest ? path.getDigest() : null, evaluateFAN(output));
+ assertValueMatches(path.stat(), path.getDigest(), evaluateFAN(output));
}
@Test
- public void testBasicArtifact() throws Throwable {
+ public void testBasicArtifact() throws Exception {
fastDigest = false;
- assertFileArtifactValueMatches(/*expectDigest=*/ true);
+ assertFileArtifactValueMatches();
}
@Test
- public void testBasicArtifactWithXattr() throws Throwable {
+ public void testBasicArtifactWithXattr() throws Exception {
fastDigest = true;
- assertFileArtifactValueMatches(/*expectDigest=*/ true);
+ assertFileArtifactValueMatches();
}
@Test
@@ -335,12 +335,10 @@
FilesetOutputSymlink.createForTesting(
PathFragment.EMPTY_FRAGMENT, PathFragment.EMPTY_FRAGMENT, PathFragment.EMPTY_FRAGMENT);
ActionExecutionValue actionExecutionValue =
- ActionExecutionValue.create(
+ ActionExecutionValue.createForTesting(
ImmutableMap.of(artifact1, metadata1, artifact3, FileArtifactValue.DEFAULT_MIDDLEMAN),
ImmutableMap.of(treeArtifact, tree),
- ImmutableList.of(filesetOutputSymlink),
- /*discoveredModules=*/ null,
- /*shareable=*/ false);
+ ImmutableList.of(filesetOutputSymlink));
new SerializationTester(actionExecutionValue)
.addDependency(FileSystem.class, root.getFileSystem())
.addDependency(
@@ -430,11 +428,11 @@
}
}
- private FileArtifactValue evaluateFAN(Artifact artifact) throws Throwable {
+ private FileArtifactValue evaluateFAN(Artifact artifact) throws Exception {
return ((FileArtifactValue) evaluateArtifactValue(artifact));
}
- private SkyValue evaluateArtifactValue(Artifact artifact) throws Throwable {
+ private SkyValue evaluateArtifactValue(Artifact artifact) throws Exception {
SkyKey key = Artifact.key(artifact);
EvaluationResult<SkyValue> result = evaluate(ImmutableList.of(key).toArray(new SkyKey[0]));
if (result.hasError()) {
@@ -520,8 +518,7 @@
FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW)),
null);
FileArtifactValue withDigest =
- FileArtifactValue.createFromInjectedDigest(
- noDigest, path.getDigest(), !output.isConstantMetadata());
+ FileArtifactValue.createFromInjectedDigest(noDigest, path.getDigest());
artifactData.put(output, withDigest);
} else {
artifactData.put(output, FileArtifactValue.DEFAULT_MIDDLEMAN);
@@ -529,12 +526,10 @@
} catch (IOException e) {
throw new IllegalStateException(e);
}
- return ActionExecutionValue.create(
+ return ActionExecutionValue.createForTesting(
ImmutableMap.copyOf(artifactData),
ImmutableMap.copyOf(treeArtifactData),
- /*outputSymlinks=*/ null,
- /*discoveredModules=*/ null,
- /*shareable=*/ true);
+ /*outputSymlinks=*/ null);
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java
index eac821f..0435a19 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java
@@ -65,34 +65,16 @@
new EqualsTester()
.addEqualityGroup(
FileArtifactValue.createForNormalFile(
- toBytes("00112233445566778899AABBCCDDEEFF"),
- /*proxy=*/ null,
- 1L,
- /*isShareable=*/ true),
+ toBytes("00112233445566778899AABBCCDDEEFF"), /*proxy=*/ null, 1L),
FileArtifactValue.createForNormalFile(
- toBytes("00112233445566778899AABBCCDDEEFF"),
- /*proxy=*/ null,
- 1L,
- /*isShareable=*/ true))
+ toBytes("00112233445566778899AABBCCDDEEFF"), /*proxy=*/ null, 1L))
.addEqualityGroup(
FileArtifactValue.createForNormalFile(
- toBytes("00112233445566778899AABBCCDDEEFF"),
- /*proxy=*/ null,
- 2L,
- /*isShareable=*/ true))
+ toBytes("00112233445566778899AABBCCDDEEFF"), /*proxy=*/ null, 2L))
.addEqualityGroup(FileArtifactValue.createForDirectoryWithMtime(1))
.addEqualityGroup(
FileArtifactValue.createForNormalFile(
- toBytes("FFFFFF00000000000000000000000000"),
- /*proxy=*/ null,
- 1L,
- /*isShareable=*/ true))
- .addEqualityGroup(
- FileArtifactValue.createForNormalFile(
- toBytes("FFFFFF00000000000000000000000000"),
- /*proxy=*/ null,
- 1L,
- /*isShareable=*/ false))
+ toBytes("FFFFFF00000000000000000000000000"), /*proxy=*/ null, 1L))
.addEqualityGroup(
FileArtifactValue.createForDirectoryWithMtime(2),
FileArtifactValue.createForDirectoryWithMtime(2))
@@ -152,7 +134,7 @@
assertThrows(
"mtime for non-empty file should not be stored.",
UnsupportedOperationException.class,
- () -> value.getModifiedTime());
+ value::getModifiedTime);
}
@Test
@@ -174,12 +156,12 @@
assertThrows(
"mtime for non-empty file should not be stored.",
UnsupportedOperationException.class,
- () -> value.getModifiedTime());
+ value::getModifiedTime);
}
@Test
public void testIOException() throws Exception {
- final IOException exception = new IOException("beep");
+ IOException exception = new IOException("beep");
FileSystem fs =
new InMemoryFileSystem(DigestHashFunction.SHA256) {
@Override
@@ -188,6 +170,7 @@
}
@Override
+ @SuppressWarnings("UnsynchronizedOverridesSynchronized")
protected byte[] getFastDigest(PathFragment path) throws IOException {
throw exception;
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index 856a10c..05d81af 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -181,7 +181,7 @@
new ExternalPackageFunction(BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER));
differencer = new SequencedRecordingDifferencer();
- evaluator = new InMemoryMemoizingEvaluator(skyFunctions.build(), differencer);
+ evaluator = new InMemoryMemoizingEvaluator(skyFunctions.buildOrThrow(), differencer);
driver = new SequentialBuildDriver(evaluator);
PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
@@ -930,39 +930,28 @@
FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW)),
null);
FileArtifactValue withDigest =
- FileArtifactValue.createFromInjectedDigest(
- noDigest, path.getDigest(), !output.isConstantMetadata());
+ FileArtifactValue.createFromInjectedDigest(noDigest, path.getDigest());
artifactData.put(output, withDigest);
} catch (IOException e) {
throw new IllegalStateException(e);
}
}
- return ActionExecutionValue.create(
+ return ActionExecutionValue.createForTesting(
ImmutableMap.copyOf(artifactData),
/*treeArtifactData=*/ ImmutableMap.of(),
- /*outputSymlinks=*/ null,
- /*discoveredModules=*/ null,
- /*shareable=*/ true);
+ /*outputSymlinks=*/ null);
}
private static ActionExecutionValue actionValueWithTreeArtifact(
SpecialArtifact output, TreeArtifactValue tree) {
- return ActionExecutionValue.create(
- ImmutableMap.of(),
- ImmutableMap.of(output, tree),
- /*outputSymlinks=*/ null,
- /*discoveredModules=*/ null,
- /*shareable=*/ true);
+ return ActionExecutionValue.createForTesting(
+ ImmutableMap.of(), ImmutableMap.of(output, tree), /*outputSymlinks=*/ null);
}
private static ActionExecutionValue actionValueWithRemoteArtifact(
Artifact output, RemoteFileArtifactValue value) {
- return ActionExecutionValue.create(
- ImmutableMap.of(output, value),
- ImmutableMap.of(),
- /*outputSymlinks=*/ null,
- /*discoveredModules=*/ null,
- /*actionDependsOnBuildId=*/ false);
+ return ActionExecutionValue.createForTesting(
+ ImmutableMap.of(output, value), ImmutableMap.of(), /*outputSymlinks=*/ null);
}
private RemoteFileArtifactValue createRemoteFileArtifactValue(String contents) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTestBase.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTestBase.java
index ba07124..8f60dac 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTestBase.java
@@ -108,12 +108,10 @@
Map<Artifact, TreeArtifactValue> treeArtifactData = new HashMap<>();
treeArtifacts.injectTo(treeArtifactData::put);
- return ActionExecutionValue.create(
+ return ActionExecutionValue.createForTesting(
/*artifactData=*/ ImmutableMap.of(),
ImmutableMap.copyOf(treeArtifactData),
- /*outputSymlinks=*/ null,
- /*discoveredModules=*/ null,
- /*shareable=*/ true);
+ /*outputSymlinks=*/ null);
}
private static FileArtifactValue createMetadataFromFileSystem(Artifact artifact)
@@ -122,8 +120,7 @@
FileArtifactValue noDigest =
ActionMetadataHandler.fileArtifactValueFromArtifact(
artifact, FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW)), null);
- return FileArtifactValue.createFromInjectedDigest(
- noDigest, path.getDigest(), !artifact.isConstantMetadata());
+ return FileArtifactValue.createFromInjectedDigest(noDigest, path.getDigest());
}
void writeFile(Path path, String... lines) throws IOException {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
index c95ba3f..6681cfd 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
-import com.google.common.base.Predicate;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -42,7 +41,6 @@
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.FileStatusWithDigestAdapter;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
@@ -107,7 +105,7 @@
@Test
public void testEmptyTreeArtifacts() throws Exception {
- TreeArtifactValue value = doTestTreeArtifacts(ImmutableList.<PathFragment>of());
+ TreeArtifactValue value = doTestTreeArtifacts(ImmutableList.of());
// Additional test, only for this test method: we expect the FileArtifactValue is equal to
// the digest [0]
assertThat(value.getMetadata().getDigest()).isEqualTo(value.getDigest());
@@ -140,14 +138,8 @@
ImmutableList.of(PathFragment.create("one"), PathFragment.create("two"));
TreeArtifactValue valueOne = evaluateTreeArtifact(treeArtifact, children);
MemoizingEvaluator evaluator = driver.getGraphForTesting();
- evaluator.delete(
- new Predicate<SkyKey>() {
- @Override
- public boolean apply(SkyKey key) {
- // Delete action execution node to force our artifacts to be re-evaluated.
- return actions.contains(key.argument());
- }
- });
+ // Delete action execution node to force our artifacts to be re-evaluated.
+ evaluator.delete(key -> actions.contains(key.argument()));
TreeArtifactValue valueTwo = evaluateTreeArtifact(treeArtifact, children);
assertThat(valueOne.getDigest()).isNotSameInstanceAs(valueTwo.getDigest());
assertThat(valueOne).isEqualTo(valueTwo);
@@ -209,7 +201,7 @@
}
private static void file(Path path, String contents) throws Exception {
- FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ path.getParentDirectory().createDirectoryAndParents();
writeFile(path, contents);
}
@@ -223,7 +215,7 @@
ALL_OWNER,
SpecialArtifactType.TREE);
actions.add(new DummyAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), output));
- FileSystemUtils.createDirectoryAndParents(fullPath);
+ fullPath.createDirectoryAndParents();
return output;
}
@@ -282,20 +274,17 @@
FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW)),
null);
FileArtifactValue withDigest =
- FileArtifactValue.createFromInjectedDigest(
- noDigest, path.getDigest(), !output.isConstantMetadata());
+ FileArtifactValue.createFromInjectedDigest(noDigest, path.getDigest());
tree.putChild(suboutput, withDigest);
} catch (IOException e) {
throw new SkyFunctionException(e, Transience.TRANSIENT) {};
}
}
- return ActionExecutionValue.create(
+ return ActionExecutionValue.createForTesting(
/*artifactData=*/ ImmutableMap.of(),
ImmutableMap.of(output, tree.build()),
- /*outputSymlinks=*/ null,
- /*discoveredModules=*/ null,
- /*shareable=*/ true);
+ /*outputSymlinks=*/ null);
}
}
}
diff --git a/src/test/java/com/google/devtools/build/skyframe/GraphTester.java b/src/test/java/com/google/devtools/build/skyframe/GraphTester.java
index a94b501..a6d4d97 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphTester.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphTester.java
@@ -26,7 +26,6 @@
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.Pair;
-import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import java.util.HashMap;
import java.util.LinkedHashMap;
@@ -197,7 +196,7 @@
}
public TestFunction addDependency(SkyKey key) {
- deps.add(Pair.<SkyKey, SkyValue>of(key, null));
+ deps.add(Pair.of(key, null));
return this;
}
@@ -324,9 +323,7 @@
functionMap.put(functionName, function);
}
- /**
- * Simple value class that stores strings.
- */
+ /** Simple value class that stores strings. */
public static class StringValue implements SkyValue {
protected final String value;
@@ -340,6 +337,9 @@
@Override
public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
if (!(o instanceof StringValue)) {
return false;
}
@@ -353,7 +353,7 @@
@Override
public String toString() {
- return "StringValue: " + getValue();
+ return "StringValue: " + value;
}
public static StringValue of(String string) {
@@ -384,18 +384,6 @@
}
}
- /** An unshareable version of {@link StringValue}. */
- public static final class UnshareableStringValue extends StringValue {
- public UnshareableStringValue(String value) {
- super(value);
- }
-
- @Override
- public boolean dataIsShareable() {
- return false;
- }
- }
-
/**
* A callback interface to provide the value computation.
*/
@@ -405,32 +393,20 @@
throws InterruptedException;
}
- public static final ValueComputer COPY = new ValueComputer() {
- @Override
- public SkyValue compute(Map<SkyKey, SkyValue> deps, SkyFunction.Environment env) {
- return Iterables.getOnlyElement(deps.values());
- }
- };
+ public static final ValueComputer COPY = (deps, env) -> Iterables.getOnlyElement(deps.values());
- public static final ValueComputer CONCATENATE = new ValueComputer() {
- @Override
- public SkyValue compute(Map<SkyKey, SkyValue> deps, SkyFunction.Environment env) {
- StringBuilder result = new StringBuilder();
- for (SkyValue value : deps.values()) {
- result.append(((StringValue) value).value);
- }
- return new StringValue(result.toString());
- }
- };
+ public static final ValueComputer CONCATENATE =
+ (deps, env) -> {
+ StringBuilder result = new StringBuilder();
+ for (SkyValue value : deps.values()) {
+ result.append(((StringValue) value).value);
+ }
+ return new StringValue(result.toString());
+ };
- public static ValueComputer formatter(final SkyKey key, final String format) {
- return new ValueComputer() {
- @Override
- public SkyValue compute(Map<SkyKey, SkyValue> deps, Environment env)
- throws InterruptedException {
- return StringValue.of(String.format(format, StringValue.from(deps.get(key)).getValue()));
- }
- };
+ public static ValueComputer formatter(SkyKey key, String format) {
+ return (deps, env) ->
+ StringValue.of(String.format(format, StringValue.from(deps.get(key)).getValue()));
}
@AutoCodec.VisibleForSerialization
