Optimize memory usage of `RegularFileValue`... by removing it.
`RegularFileValue` (i.e. the path's real path is itself, and it's an existing file) is the common case situation for `FileValue`. In this situation we don't actually need to spend shallow heap to store a reference to the original `RootedPath` in order to service `FileValue#realRootedPath` and `FileValue#logicalChainDuringResolution`; instead we refactor all callers to pass in the original `RootedPath`. After making this change we notice that `RegularFileValue` is now just a wrapper around `FileStateValue`, so then we remove the need for the wrapper by turning `RegularFileValue` into an abstract class (an interface would also work), and then having `FileStateValue` extend it.
PiperOrigin-RevId: 655774211
Change-Id: Icf1684c85c605a267a85c7917bb53dffaeb85fd7
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java
index 245e9ed..dea15d0 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java
@@ -19,6 +19,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
+import com.google.devtools.build.lib.actions.FileValue.RegularFileValue;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.io.InconsistentFilesystemException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
@@ -34,7 +35,6 @@
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.lib.vfs.XattrProvider;
-import com.google.devtools.build.skyframe.SkyValue;
import java.io.IOException;
import java.util.Arrays;
import java.util.Objects;
@@ -57,9 +57,14 @@
* com.google.devtools.build.lib.skyframe.FileFunction}. Instead, {@link FileValue} should be used
* by {@link com.google.devtools.build.skyframe.SkyFunction} consumers that care about files.
*
+ * <p>The common case for {@link FileValue} is {@link RegularFileValue} (i.e. the path's real path
+ * is itself, and it's an existing file). As a memory optimization for this common case, we have
+ * {@link FileStateValue} be a {@link RegularFileValue} so that we don't need a wrapper object for
+ * the value of the corresponding {@link FileValue} node.
+ *
* <p>All subclasses must implement {@link #equals} and {@link #hashCode} properly.
*/
-public abstract class FileStateValue implements HasDigest, SkyValue {
+public abstract class FileStateValue extends RegularFileValue implements HasDigest {
@SerializationConstant
public static final DirectoryFileStateValue DIRECTORY_FILE_STATE_NODE =
new DirectoryFileStateValue();
@@ -205,6 +210,11 @@
return rootedPath;
}
+ @Override
+ public FileStateValue realFileStateValue() {
+ return this;
+ }
+
public abstract FileStateType getType();
/** Returns the target of the symlink, or throws an exception if this is not a symlink. */
@@ -212,7 +222,8 @@
throw new IllegalStateException();
}
- long getSize() {
+ @Override
+ public long getSize() {
throw new IllegalStateException();
}
@@ -404,7 +415,7 @@
}
@Override
- long getSize() {
+ public long getSize() {
return 0;
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
index a1a5253..698c062 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
@@ -41,26 +41,26 @@
* File values for missing files will be created on purpose in order to facilitate incremental
* builds in the case those files have reappeared.
*
- * <p>This class contains the relevant metadata for a file, although not the contents. Note that
- * since a FileValue doesn't store its corresponding SkyKey, it's possible for the FileValues for
- * two different paths to be the same.
+ * <p>This interface encapsulates the relevant metadata for a file, although not the contents. Note
+ * that since a FileValue doesn't necessarily store its corresponding SkyKey, it's possible for the
+ * FileValues for two different paths to be the same.
*
* <p>This should not be used for build outputs; use {@link ArtifactSkyKey} to create keys for
* those.
*/
@Immutable
@ThreadSafe
-public abstract class FileValue implements SkyValue {
+public interface FileValue extends SkyValue {
// Depends non-hermetically on package path, but that is under the control of a flag, so use
// semi-hermetic.
public static final SkyFunctionName FILE = SkyFunctionName.createSemiHermetic("FILE");
- public boolean exists() {
+ default boolean exists() {
return realFileStateValue().getType() != FileStateType.NONEXISTENT;
}
/** Returns true if the original path is a symlink; the target path can never be a symlink. */
- public boolean isSymlink() {
+ default boolean isSymlink() {
return false;
}
@@ -68,7 +68,7 @@
* Returns true if this value corresponds to a file or symlink to an existing regular or special
* file. If so, its parent directory is guaranteed to exist.
*/
- public boolean isFile() {
+ default boolean isFile() {
return realFileStateValue().getType() == FileStateType.REGULAR_FILE
|| realFileStateValue().getType() == FileStateType.SPECIAL_FILE;
}
@@ -77,7 +77,7 @@
* Returns true if this value corresponds to a special file or symlink to a special file. If so,
* its parent directory is guaranteed to exist.
*/
- public boolean isSpecialFile() {
+ default boolean isSpecialFile() {
return realFileStateValue().getType() == FileStateType.SPECIAL_FILE;
}
@@ -85,7 +85,7 @@
* Returns true if the file is a directory or a symlink to an existing directory. If so, its
* parent directory is guaranteed to exist.
*/
- public boolean isDirectory() {
+ default boolean isDirectory() {
return realFileStateValue().getType() == FileStateType.DIRECTORY;
}
@@ -99,28 +99,28 @@
* -- this information is only needed for resolving ancestors, and an existing file or a
* non-existent directory has no descendants, by definition.
*/
- public abstract ImmutableList<RootedPath> logicalChainDuringResolution();
+ ImmutableList<RootedPath> logicalChainDuringResolution(RootedPath initialRootedPath);
/**
* If a symlink pointing back to its own ancestor was encountered during the resolution of this
* {@link FileValue}, returns the path to it. Otherwise, returns null.
*/
- public abstract ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain();
+ ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain();
/**
* If a symlink pointing back to its own ancestor was encountered during the resolution of this
* {@link FileValue}, returns the symlinks in the cycle. Otherwise, returns null.
*/
- public abstract ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain();
+ ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain();
/**
* Returns the real rooted path of the file, taking ancestor symlinks into account. For example,
* the rooted path ['root']/['a/b'] is really ['root']/['c/b'] if 'a' is a symlink to 'c'. Note
* that ancestor symlinks outside the root boundary are not taken into consideration.
*/
- public abstract RootedPath realRootedPath();
+ RootedPath realRootedPath(RootedPath initialRootedPath);
- public abstract FileStateValue realFileStateValue();
+ FileStateValue realFileStateValue();
/**
* Returns the unresolved link target if {@link #isSymlink()}.
@@ -129,31 +129,31 @@
* example could be a build rule that copies a set of input files to the output directory, but
* upon encountering symbolic links it can decide between copying or following them.
*/
- public PathFragment getUnresolvedLinkTarget() {
+ default PathFragment getUnresolvedLinkTarget() {
throw new IllegalStateException(this.toString());
}
- public long getSize() {
+ default long getSize() {
Preconditions.checkState(isFile(), this);
return realFileStateValue().getSize();
}
@Nullable
- public byte[] getDigest() {
+ default byte[] getDigest() {
Preconditions.checkState(isFile(), this);
return realFileStateValue().getDigest();
}
/** Returns a key for building a file value for the given root-relative path. */
@ThreadSafe
- public static Key key(RootedPath rootedPath) {
+ static Key key(RootedPath rootedPath) {
return Key.create(rootedPath);
}
/** Key type for FileValue. */
@VisibleForSerialization
@AutoCodec
- public static class Key extends AbstractSkyKey<RootedPath> {
+ class Key extends AbstractSkyKey<RootedPath> {
private static final SkyKeyInterner<Key> interner = SkyKey.newInterner();
private Key(RootedPath arg) {
@@ -185,7 +185,7 @@
* Only intended to be used by {@link com.google.devtools.build.lib.skyframe.FileFunction}. Should
* not be used for symlink cycles.
*/
- public static FileValue value(
+ static FileValue value(
ImmutableList<RootedPath> logicalChainDuringResolution,
ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain,
ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain,
@@ -209,7 +209,7 @@
"logicalChainDuringResolution: %s, originalRootedPath: %s",
logicalChainDuringResolution,
originalRootedPath);
- return new RegularFileValue(originalRootedPath, fileStateValueFromAncestors);
+ return fileStateValueFromAncestors;
}
boolean shouldStoreChain =
@@ -253,25 +253,14 @@
}
/**
- * Implementation of {@link FileValue} for paths whose fully resolved path is the same as the
- * requested path. For example, this is the case for the path "foo/bar/baz" if neither 'foo' nor
- * 'foo/bar' nor 'foo/bar/baz' are symlinks.
+ * A {@link FileValue} for paths whose fully resolved path is the same as the requested path. For
+ * example, this is the case for the path "foo/bar/baz" if neither 'foo' nor 'foo/bar' nor
+ * 'foo/bar/baz' are symlinks.
*/
- @VisibleForTesting
- public static final class RegularFileValue extends FileValue {
-
- private final RootedPath rootedPath;
- private final FileStateValue fileStateValue;
-
- @VisibleForTesting
- public RegularFileValue(RootedPath rootedPath, FileStateValue fileStateValue) {
- this.rootedPath = Preconditions.checkNotNull(rootedPath);
- this.fileStateValue = Preconditions.checkNotNull(fileStateValue);
- }
-
+ abstract class RegularFileValue implements FileValue {
@Override
- public ImmutableList<RootedPath> logicalChainDuringResolution() {
- return ImmutableList.of(rootedPath);
+ public ImmutableList<RootedPath> logicalChainDuringResolution(RootedPath initialRootedPath) {
+ return ImmutableList.of(initialRootedPath);
}
@Nullable
@@ -287,34 +276,8 @@
}
@Override
- public RootedPath realRootedPath() {
- return rootedPath;
- }
-
- @Override
- public FileStateValue realFileStateValue() {
- return fileStateValue;
- }
-
- @Override
- public boolean equals(Object obj) {
- if (obj == null) {
- return false;
- }
- if (!(obj instanceof RegularFileValue other)) {
- return false;
- }
- return rootedPath.equals(other.rootedPath) && fileStateValue.equals(other.fileStateValue);
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(rootedPath, fileStateValue);
- }
-
- @Override
- public String toString() {
- return String.format("non-symlink (path=%s, state=%s)", rootedPath, fileStateValue);
+ public RootedPath realRootedPath(RootedPath initialRootedPath) {
+ return initialRootedPath;
}
}
@@ -323,8 +286,7 @@
* required traversing a symlink chain caused by a symlink pointing to its own ancestor but which
* eventually points to a real file.
*/
- @VisibleForTesting
- public static class DifferentRealPathFileValueWithUnboundedAncestorExpansion
+ class DifferentRealPathFileValueWithUnboundedAncestorExpansion
extends DifferentRealPathFileValueWithStoredChain {
protected final ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain;
protected final ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain;
@@ -401,7 +363,7 @@
* path "foo/bar/baz" if at least one of {'foo', 'foo/bar'} is a symlink but 'foo/bar/baz' not.
*/
@VisibleForTesting
- public static class DifferentRealPathFileValueWithStoredChain extends FileValue {
+ class DifferentRealPathFileValueWithStoredChain implements FileValue {
protected final RootedPath realRootedPath;
protected final FileStateValue realFileStateValue;
protected final ImmutableList<RootedPath> logicalChainDuringResolution;
@@ -417,7 +379,7 @@
}
@Override
- public RootedPath realRootedPath() {
+ public RootedPath realRootedPath(RootedPath initialRootedPath) {
return realRootedPath;
}
@@ -427,7 +389,7 @@
}
@Override
- public ImmutableList<RootedPath> logicalChainDuringResolution() {
+ public ImmutableList<RootedPath> logicalChainDuringResolution(RootedPath initialRootedPath) {
return logicalChainDuringResolution;
}
@@ -475,7 +437,7 @@
* #logicalChainDuringResolution}.
*/
@VisibleForTesting
- public static class DifferentRealPathFileValueWithoutStoredChain extends FileValue {
+ class DifferentRealPathFileValueWithoutStoredChain implements FileValue {
protected final RootedPath realRootedPath;
protected final FileStateValue realFileStateValue;
@@ -487,7 +449,7 @@
}
@Override
- public RootedPath realRootedPath() {
+ public RootedPath realRootedPath(RootedPath initialRootedPath) {
return realRootedPath;
}
@@ -497,7 +459,7 @@
}
@Override
- public ImmutableList<RootedPath> logicalChainDuringResolution() {
+ public ImmutableList<RootedPath> logicalChainDuringResolution(RootedPath initialRootedPath) {
throw new IllegalStateException(this.toString());
}
@@ -543,7 +505,7 @@
* by a symlink pointing to its own ancestor and which eventually points to a symlink.
*/
@VisibleForTesting
- public static final class SymlinkFileValueWithUnboundedAncestorExpansion
+ final class SymlinkFileValueWithUnboundedAncestorExpansion
extends SymlinkFileValueWithStoredChain {
private final ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain;
private final ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain;
@@ -620,8 +582,7 @@
/** Implementation of {@link FileValue} for paths that are themselves symlinks. */
@VisibleForTesting
- public static class SymlinkFileValueWithStoredChain
- extends DifferentRealPathFileValueWithStoredChain {
+ class SymlinkFileValueWithStoredChain extends DifferentRealPathFileValueWithStoredChain {
protected final PathFragment linkTarget;
@VisibleForTesting
@@ -677,7 +638,7 @@
* #logicalChainDuringResolution}.
*/
@VisibleForTesting
- public static final class SymlinkFileValueWithoutStoredChain
+ final class SymlinkFileValueWithoutStoredChain
extends DifferentRealPathFileValueWithoutStoredChain {
private final PathFragment linkTarget;
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java
index 6e85a9e..8d2a0a3 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java
@@ -1486,16 +1486,17 @@
if (repoCacheFriendlyPath == null) {
return;
}
+ var recordedInput = new RepoRecordedInput.File(repoCacheFriendlyPath);
+ var skyKey = recordedInput.getSkyKey(directories);
try {
- var recordedInput = new RepoRecordedInput.File(repoCacheFriendlyPath);
- FileValue fileValue =
- (FileValue) env.getValueOrThrow(recordedInput.getSkyKey(directories), IOException.class);
+ FileValue fileValue = (FileValue) env.getValueOrThrow(skyKey, IOException.class);
if (fileValue == null) {
throw new NeedsSkyframeRestartException();
}
recordedFileInputs.put(
- recordedInput, RepoRecordedInput.File.fileValueToMarkerValue(fileValue));
+ recordedInput,
+ RepoRecordedInput.File.fileValueToMarkerValue((RootedPath) skyKey.argument(), fileValue));
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/BUILD b/src/main/java/com/google/devtools/build/lib/rules/BUILD
index 62bbe82..a6a3363 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/BUILD
@@ -346,6 +346,7 @@
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value",
+ "//src/main/java/com/google/devtools/build/lib/util:pair",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/net/starlark/java/eval",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java b/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java
index aad74b6..1ed1b3b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction;
import com.google.devtools.build.lib.skyframe.PackageLookupValue;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
@@ -74,6 +75,7 @@
private abstract static class BaseFileHandler {
private final String filename;
+ private RootedPath rootedPath;
private FileValue fileValue;
private String fileContent;
@@ -112,7 +114,9 @@
Transience.PERSISTENT);
} else if (hasFile) {
- fileValue = getFileValue(rule, env);
+ Pair<RootedPath, FileValue> rootedPathAndFileValue = getFileValue(rule, env);
+ rootedPath = rootedPathAndFileValue.getFirst();
+ fileValue = rootedPathAndFileValue.getSecond();
if (env.valuesMissing()) {
return false;
}
@@ -145,14 +149,14 @@
throws RepositoryFunctionException {
if (fileValue != null) {
// Link x/FILENAME to <build_root>/x.FILENAME.
- symlinkFile(fileValue, filename, outputDirectory);
+ symlinkFile(rootedPath, fileValue, filename, outputDirectory);
try {
Label label = getFileAttributeAsLabel(rule);
recordedInputValues.put(
new RepoRecordedInput.File(
RepoRecordedInput.RepoCacheFriendlyPath.createInsideWorkspace(
label.getRepository(), label.toPathFragment())),
- RepoRecordedInput.File.fileValueToMarkerValue(fileValue));
+ RepoRecordedInput.File.fileValueToMarkerValue(rootedPath, fileValue));
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
@@ -172,7 +176,7 @@
}
@Nullable
- private FileValue getFileValue(Rule rule, Environment env)
+ private Pair<RootedPath, FileValue> getFileValue(Rule rule, Environment env)
throws RepositoryFunctionException, InterruptedException {
Label label = getFileAttributeAsLabel(rule);
SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier());
@@ -223,20 +227,23 @@
Transience.PERSISTENT);
}
- return fileValue;
+ return Pair.of(rootedFile, fileValue);
}
/**
* Symlinks a file from the local filesystem into the external repository's root.
*
+ * @param rootedPath {@link RootedPath} of the file to be linked in
* @param fileValue {@link FileValue} representing the file to be linked in
* @param outputDirectory the directory of the remote repository
* @throws RepositoryFunctionException if the file specified does not exist or cannot be linked.
*/
- private static void symlinkFile(FileValue fileValue, String filename, Path outputDirectory)
+ private static void symlinkFile(
+ RootedPath rootedPath, FileValue fileValue, String filename, Path outputDirectory)
throws RepositoryFunctionException {
Path filePath = outputDirectory.getRelative(filename);
- RepositoryFunction.createSymbolicLink(filePath, fileValue.realRootedPath().asPath());
+ RepositoryFunction.createSymbolicLink(
+ filePath, fileValue.realRootedPath(rootedPath).asPath());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java
index 8da9d50..7dc9bde 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java
@@ -305,7 +305,8 @@
* for placing in a repository marker file. The file need not exist, and can be a file or a
* directory.
*/
- public static String fileValueToMarkerValue(FileValue fileValue) throws IOException {
+ public static String fileValueToMarkerValue(RootedPath rootedPath, FileValue fileValue)
+ throws IOException {
if (fileValue.isDirectory()) {
return "DIR";
}
@@ -316,7 +317,7 @@
byte[] digest = fileValue.realFileStateValue().getDigest();
if (digest == null) {
// Fast digest not available, or it would have been in the FileValue.
- digest = fileValue.realRootedPath().asPath().getDigest();
+ digest = fileValue.realRootedPath(rootedPath).asPath().getDigest();
}
return BaseEncoding.base16().lowerCase().encode(digest);
}
@@ -331,13 +332,13 @@
public boolean isUpToDate(
Environment env, BlazeDirectories directories, @Nullable String oldValue)
throws InterruptedException {
+ var skyKey = getSkyKey(directories);
try {
- FileValue fileValue =
- (FileValue) env.getValueOrThrow(getSkyKey(directories), IOException.class);
+ FileValue fileValue = (FileValue) env.getValueOrThrow(skyKey, IOException.class);
if (fileValue == null) {
return false;
}
- return oldValue.equals(fileValueToMarkerValue(fileValue));
+ return oldValue.equals(fileValueToMarkerValue((RootedPath) skyKey.argument(), fileValue));
} catch (IOException e) {
return false;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingFunction.java
index eb01dcd..b31c0fa 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingFunction.java
@@ -38,7 +38,7 @@
return null;
}
- RootedPath realDirRootedPath = dirFileValue.realRootedPath();
+ RootedPath realDirRootedPath = dirFileValue.realRootedPath(dirRootedPath);
if (!dirFileValue.isDirectory()) {
// Recall that the directory is assumed to exist (see DirectoryListingValue#key).
throw new DirectoryListingFunctionException(new InconsistentFilesystemException(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingValue.java
index d9e9f91..02da3e5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingValue.java
@@ -95,10 +95,11 @@
static DirectoryListingValue value(RootedPath dirRootedPath, FileValue dirFileValue,
DirectoryListingStateValue realDirectoryListingStateValue) {
- return dirFileValue.realRootedPath().equals(dirRootedPath)
+ RootedPath realRootedPath = dirFileValue.realRootedPath(dirRootedPath);
+ return realRootedPath.equals(dirRootedPath)
? new RegularDirectoryListingValue(realDirectoryListingStateValue)
- : new DifferentRealPathDirectoryListingValue(dirFileValue.realRootedPath(),
- realDirectoryListingStateValue);
+ : new DifferentRealPathDirectoryListingValue(
+ realRootedPath, realDirectoryListingStateValue);
}
/** Normal {@link DirectoryListingValue}. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunction.java
index ae23c37..80ad4ab 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunction.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.util.Fingerprint;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -53,7 +54,8 @@
.collect(toImmutableSet());
// Turn each entry into a FileValue.
- ImmutableList<FileValue> fileValues = getFileValues(env, sortedDirents, rootedPath);
+ ImmutableList<Pair<RootedPath, FileValue>> fileValues =
+ getFileValues(env, sortedDirents, rootedPath);
if (fileValues == null) {
return null;
}
@@ -70,13 +72,15 @@
fp.addStrings(sortedDirents);
fp.addStrings(subDirTreeDigests);
try {
- for (FileValue fileValue : fileValues) {
+ for (Pair<RootedPath, FileValue> rootedPathAndFileValue : fileValues) {
+ RootedPath direntRootedPath = rootedPathAndFileValue.getFirst();
+ FileValue fileValue = rootedPathAndFileValue.getSecond();
fp.addInt(fileValue.realFileStateValue().getType().ordinal());
if (fileValue.isFile()) {
byte[] digest = fileValue.realFileStateValue().getDigest();
if (digest == null) {
// Fast digest not available, or it would have been in the FileValue.
- digest = fileValue.realRootedPath().asPath().getDigest();
+ digest = fileValue.realRootedPath(direntRootedPath).asPath().getDigest();
}
fp.addBytes(digest);
}
@@ -89,7 +93,7 @@
}
@Nullable
- private static ImmutableList<FileValue> getFileValues(
+ private static ImmutableList<Pair<RootedPath, FileValue>> getFileValues(
Environment env, ImmutableSet<String> sortedDirents, RootedPath rootedPath)
throws InterruptedException {
ImmutableSet<FileValue.Key> fileValueKeys =
@@ -105,10 +109,9 @@
if (env.valuesMissing()) {
return null;
}
- ImmutableList<FileValue> fileValues =
+ ImmutableList<Pair<RootedPath, FileValue>> fileValues =
fileValueKeys.stream()
- .map(result::get)
- .map(FileValue.class::cast)
+ .map(k -> Pair.of((RootedPath) k.argument(), (FileValue) result.get(k)))
.collect(toImmutableList());
if (env.valuesMissing()) {
return null;
@@ -118,11 +121,12 @@
@Nullable
private static ImmutableList<String> getSubDirTreeDigests(
- Environment env, ImmutableList<FileValue> fileValues) throws InterruptedException {
+ Environment env, ImmutableList<Pair<RootedPath, FileValue>> fileValues)
+ throws InterruptedException {
ImmutableSet<SkyKey> dirTreeDigestValueKeys =
fileValues.stream()
- .filter(FileValue::isDirectory)
- .map(fv -> DirectoryTreeDigestValue.key(fv.realRootedPath()))
+ .filter(p -> p.getSecond().isDirectory())
+ .map(p -> DirectoryTreeDigestValue.key(p.getSecond().realRootedPath(p.getFirst())))
.collect(toImmutableSet());
SkyframeLookupResult result = env.getValuesAndExceptions(dirTreeDigestValueKeys);
if (env.valuesMissing()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
index 4d7d97d..1439311 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
@@ -196,14 +196,19 @@
}
RootedPath rootedPathFromAncestors =
- getChild(parentFileValue.realRootedPath(), baseName, parentRootedPath, rootedPath);
+ getChild(
+ parentFileValue.realRootedPath(parentRootedPath),
+ baseName,
+ parentRootedPath,
+ rootedPath);
if (!parentFileValue.exists() || !parentFileValue.isDirectory()) {
return new PartialResolutionResult(
rootedPathFromAncestors, FileStateValue.NONEXISTENT_FILE_STATE_NODE);
}
- for (RootedPath parentPartialRootedPath : parentFileValue.logicalChainDuringResolution()) {
+ for (RootedPath parentPartialRootedPath :
+ parentFileValue.logicalChainDuringResolution(parentRootedPath)) {
checkAndNotePathSeenDuringPartialResolution(
getChild(parentPartialRootedPath, baseName, parentRootedPath, rootedPath),
symlinkResolutionState,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
index 0151bc0..6aec2b6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
@@ -386,21 +386,24 @@
}
} else {
// Stat the file.
+ RootedPath rootedPath = traversal.root().asRootedPath();
FileValue fileValue =
- (FileValue)
- env.getValueOrThrow(
- FileValue.key(traversal.root().asRootedPath()), IOException.class);
+ (FileValue) env.getValueOrThrow(FileValue.key(rootedPath), IOException.class);
if (env.valuesMissing()) {
return null;
}
- return toFileInfo(fileValue, env, traversal.root().asPath(), syscallCache);
+ return toFileInfo(rootedPath, fileValue, env, traversal.root().asPath(), syscallCache);
}
}
@Nullable
private static FileInfo toFileInfo(
- FileValue fileValue, Environment env, Path path, SyscallCache syscallCache)
+ RootedPath rootedPath,
+ FileValue fileValue,
+ Environment env,
+ Path path,
+ SyscallCache syscallCache)
throws IOException, InterruptedException {
if (fileValue.unboundedAncestorSymlinkExpansionChain() != null) {
SkyKey uniquenessKey =
@@ -437,7 +440,7 @@
return new FileInfo(
type,
withDigest(fileValue.realFileStateValue(), path, syscallCache),
- fileValue.realRootedPath(),
+ fileValue.realRootedPath(rootedPath),
unresolvedLinkTarget);
}
@@ -715,7 +718,12 @@
}
if (key instanceof FileValue.Key fileKey) {
FileInfo fileInfo =
- toFileInfo((FileValue) value, env, fileKey.argument().asPath(), syscallCache);
+ toFileInfo(
+ fileKey.argument(),
+ (FileValue) value,
+ env,
+ fileKey.argument().asPath(),
+ syscallCache);
if (fileInfo != null) {
childValues.add(resultForFileRoot(fileKey.argument(), fileInfo));
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/config/PlatformMappingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/config/PlatformMappingFunction.java
index fd7f68a..f27aa6c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/config/PlatformMappingFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/config/PlatformMappingFunction.java
@@ -111,7 +111,8 @@
List<String> lines;
try {
- lines = FileSystemUtils.readLines(fileValue.realRootedPath().asPath(), UTF_8);
+ lines =
+ FileSystemUtils.readLines(fileValue.realRootedPath(rootedMappingPath).asPath(), UTF_8);
} catch (IOException e) {
throw new PlatformMappingFunctionException(e);
}
