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/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);
}
