diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 972c15a05ebe8f..143931fd7f4a83 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -1999,6 +1999,7 @@ java_library( ":action_execution_value", ":detailed_exceptions", ":directory_listing_value", + ":dirents", ":package_lookup_value", ":sky_functions", ":tree_artifact_value", 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 ed943ef8dd8ba6..b723829c34e54d 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 @@ -20,6 +20,7 @@ import com.google.common.base.Verify; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; @@ -64,7 +65,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -230,12 +230,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) } // We are free to traverse this directory. - Collection dependentKeys = createRecursiveTraversalKeys(env, traversal); - if (dependentKeys == null) { - return null; - } Collection subdirTraversals = - traverseChildren(env, dependentKeys, /*inline=*/ traversal.isRootGenerated); + traverseChildren(env, traversal); if (subdirTraversals == null) { return null; } @@ -562,56 +558,14 @@ private static PkgLookupResult checkIfPackage( } } - /** - * List the directory and create {@code SkyKey}s to request contents of its children recursively. - * - *

The returned keys are of type {@link SkyFunctions#RECURSIVE_FILESYSTEM_TRAVERSAL}. - */ - @Nullable - private static Collection createRecursiveTraversalKeys( - Environment env, TraversalRequest traversal) throws InterruptedException, IOException { - // Use the traversal's path, even if it's a symlink. The contents of the directory, as listed - // in the result, must be relative to it. - Iterable dirents; - if (traversal.isRootGenerated) { - // If we're dealing with an output file, read the directory directly instead of creating - // filesystem nodes under the output tree. - List direntsCollection = - new ArrayList<>( - traversal.root.asRootedPath().asPath().readdir(Symlinks.FOLLOW)); - Collections.sort(direntsCollection); - dirents = direntsCollection; - } else { - DirectoryListingValue dirListingValue = - (DirectoryListingValue) - env.getValueOrThrow( - DirectoryListingValue.key(traversal.root.asRootedPath()), IOException.class); - if (dirListingValue == null) { - return null; - } - dirents = dirListingValue.getDirents(); - } - - List result = new ArrayList<>(); - for (Dirent dirent : dirents) { - RootedPath childPath = - RootedPath.toRootedPath( - traversal.root.asRootedPath().getRoot(), - traversal.root.asRootedPath().getRootRelativePath().getRelative(dirent.getName())); - TraversalRequest childTraversal = traversal.forChildEntry(childPath); - result.add(childTraversal); - } - return result; - } - /** * Creates result for a dangling symlink. * * @param linkName path to the symbolic link * @param info the {@link FileInfo} associated with the link file */ - private static RecursiveFilesystemTraversalValue resultForDanglingSymlink(RootedPath linkName, - FileInfo info) { + private static RecursiveFilesystemTraversalValue resultForDanglingSymlink( + RootedPath linkName, FileInfo info) { Preconditions.checkState(info.type.isSymlink() && !info.type.exists(), "{%s} {%s}", linkName, info.type); return RecursiveFilesystemTraversalValue.of( @@ -638,8 +592,10 @@ private static RecursiveFilesystemTraversalValue resultForFileRoot(RootedPath pa } } - private static RecursiveFilesystemTraversalValue resultForDirectory(TraversalRequest traversal, - FileInfo rootInfo, Collection subdirTraversals) { + private static RecursiveFilesystemTraversalValue resultForDirectory( + TraversalRequest traversal, + FileInfo rootInfo, + Collection subdirTraversals) { // Collect transitive closure of files in subdirectories. NestedSetBuilder paths = NestedSetBuilder.stableOrder(); for (RecursiveFilesystemTraversalValue child : subdirTraversals) { @@ -677,22 +633,56 @@ private static HasDigest hashDirectorySymlink( return new HasDigest.ByteStringDigest(result); } - /** - * Requests Skyframe to compute the dependent values and returns them. - * - *

The keys must all be {@link SkyFunctions#RECURSIVE_FILESYSTEM_TRAVERSAL} keys. - */ + /** Requests Skyframe to compute the dependent values and returns them. */ @Nullable private Collection traverseChildren( - Environment env, Iterable keys, boolean inline) - throws InterruptedException, RecursiveFilesystemTraversalFunctionException { - Map values; - if (inline) { + Environment env, TraversalRequest traversal) + throws InterruptedException, RecursiveFilesystemTraversalFunctionException, IOException { + // Use the traversal's path, even if it's a symlink. The contents of the directory, as listed + // in the result, must be relative to it. + Iterable direntsIterable; + int direntsSize; + if (traversal.isRootGenerated) { + // If we're dealing with an output file, read the directory directly instead of creating + // filesystem nodes under the output tree. + List direntsCollection = + new ArrayList<>(traversal.root.asRootedPath().asPath().readdir(Symlinks.FOLLOW)); + Collections.sort(direntsCollection); + direntsIterable = direntsCollection; + direntsSize = direntsCollection.size(); + } else { + DirectoryListingValue dirListingValue = + (DirectoryListingValue) + env.getValueOrThrow( + DirectoryListingValue.key(traversal.root.asRootedPath()), IOException.class); + if (dirListingValue == null) { + return null; + } + Dirents dirents = dirListingValue.getDirents(); + direntsSize = dirents.size(); + direntsIterable = dirents; + } + + ImmutableList.Builder keysBuilder = + ImmutableList.builderWithExpectedSize(direntsSize); + for (Dirent dirent : direntsIterable) { + RootedPath childPath = + RootedPath.toRootedPath( + traversal.root.asRootedPath().getRoot(), + traversal.root.asRootedPath().getRootRelativePath().getRelative(dirent.getName())); + TraversalRequest childTraversal = traversal.forChildEntry(childPath); + keysBuilder.add(childTraversal); + } + ImmutableList keys = keysBuilder.build(); + if (keys == null) { + return null; + } + Map values = Maps.newHashMapWithExpectedSize(keys.size()); + if (traversal.isRootGenerated) { // Don't create Skyframe nodes for a recursive traversal over the output tree. // Instead, inline the recursion in the top-level request. - values = new HashMap<>(); - for (SkyKey depKey : keys) { - values.put(depKey, compute(depKey, env)); + for (TraversalRequest traversalRequest : keys) { + values.put(traversalRequest, compute(traversalRequest, env)); } } else { values = env.getValues(keys);