From 2217b525c1d76add2dceb30cdd9804daa5e01a32 Mon Sep 17 00:00:00 2001 From: djasper Date: Mon, 23 Nov 2020 03:46:42 -0800 Subject: [PATCH] Reduce the number of stats performed when creating the output directories. The Path.createDirectoryAndParents() function generally walks up the given output path, static for each path segment until it finds one that exists, the walks back down creating the directories. checkSymlinks does the same walk checking for symlinks along the way. Combining the two functions conserves ~half of the stats (more in reality as an in-memory cache is used). No functional changes intended. RELNOTES: None. PiperOrigin-RevId: 343821245 --- .../lib/skyframe/SkyframeActionExecutor.java | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) 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 7ac26413498e2d..57507867098d03 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 @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.common.flogger.GoogleLogger; @@ -111,6 +112,7 @@ import java.io.Closeable; import java.io.FileNotFoundException; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -1260,27 +1262,42 @@ private void createActionFsOutputDirectories(Action action, ActionExecutionConte } /** - * Ensure that no symlinks exists between the output root and the output file. These are all - * expected to be regular directories. Violations of this expectations can only come from state - * left behind by previous invocations or external filesystem mutation. + * Create an output directory and ensure that no symlinks exists between the output root and the + * output file. These are all expected to be regular directories. Violations of this expectations + * can only come from state left behind by previous invocations or external filesystem mutation. */ - private void symlinkCheck( + private void createAndCheckForSymlinks( final Path dir, final Artifact outputFile, ActionExecutionContext context) throws IOException { PathFragment root = outputFile.getRoot().getRoot().asPath().asFragment(); Path curDir = context.getPathResolver().convertPath(dir); Set checkDirs = new HashSet<>(); + List dirsToCreate = new ArrayList<>(); + + // If the output root has not been created yet, do so now. + if (!knownRegularDirectories.contains(root)) { + outputFile.getRoot().getRoot().asPath().createDirectoryAndParents(); + knownRegularDirectories.add(root); + } + while (!curDir.asFragment().equals(root)) { // Fast path: Somebody already checked that this is a regular directory this invocation. if (knownRegularDirectories.contains(curDir.asFragment())) { break; } - if (!curDir.isDirectory(Symlinks.NOFOLLOW)) { + FileStatus stat = curDir.statNullable(Symlinks.NOFOLLOW); + if (stat != null && !stat.isDirectory()) { throw new IOException(curDir + " is not a regular directory"); } + if (stat == null) { + dirsToCreate.add(curDir); + } checkDirs.add(curDir.asFragment()); curDir = curDir.getParentDirectory(); } + for (Path path : Lists.reverse(dirsToCreate)) { + path.createDirectory(); + } // Defer adding to known regular directories until we've checked all parent directories. knownRegularDirectories.addAll(checkDirs); @@ -1300,10 +1317,7 @@ private void createOutputDirectories(Action action, ActionExecutionContext conte if (done.add(outputDir)) { try { - if (!knownRegularDirectories.contains(outputDir.asFragment())) { - outputDir.createDirectoryAndParents(); - symlinkCheck(outputDir, outputFile, context); - } + createAndCheckForSymlinks(outputDir, outputFile, context); continue; } catch (IOException e) { /* Fall through to plan B. */