Remove cached version of FileSystemUtils#createDirectoryAndParents.
Only the sandbox uses the cached version. We can let it do its own caching of the top-level directory. It won't help with higher-level directories, but if there are a lot of inputs in the same directory we'll avoid I/O there.
PiperOrigin-RevId: 179830651
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java
index 0beed3f..80db422 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java
@@ -78,7 +78,7 @@
public void createFileSystem() throws IOException {
Set<Path> createdDirs = new HashSet<>();
cleanFileSystem(inputs.keySet());
- FileSystemUtils.createDirectoryAndParentsWithCache(createdDirs, sandboxExecRoot);
+ createDirectoryAndParentsWithCache(createdDirs, sandboxExecRoot);
createParentDirectoriesForInputs(createdDirs, inputs.keySet());
createInputs(inputs);
createWritableDirectories(createdDirs, writableDirs);
@@ -126,7 +126,7 @@
Path dir = sandboxExecRoot.getRelative(inputPath).getParentDirectory();
Preconditions.checkArgument(
dir.startsWith(sandboxExecRoot), "Bad relative path: '%s'", inputPath);
- FileSystemUtils.createDirectoryAndParentsWithCache(createdDirs, dir);
+ createDirectoryAndParentsWithCache(createdDirs, dir);
}
}
@@ -156,7 +156,7 @@
throws IOException {
for (Path writablePath : writableDirs) {
if (writablePath.startsWith(sandboxExecRoot)) {
- FileSystemUtils.createDirectoryAndParentsWithCache(createdDirs, writablePath);
+ createDirectoryAndParentsWithCache(createdDirs, writablePath);
}
}
}
@@ -165,7 +165,7 @@
private void createDirectoriesForOutputs(Set<Path> createdDirs, Collection<PathFragment> outputs)
throws IOException {
for (PathFragment output : outputs) {
- FileSystemUtils.createDirectoryAndParentsWithCache(
+ createDirectoryAndParentsWithCache(
createdDirs, sandboxExecRoot.getRelative(output.getParentDirectory()));
}
}
@@ -208,4 +208,11 @@
// on here.
}
}
+
+ private static void createDirectoryAndParentsWithCache(Set<Path> cache, Path dir)
+ throws IOException {
+ if (cache.add(dir)) {
+ FileSystemUtils.createDirectoryAndParents(dir);
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
index 064e4b2..e42abdf 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java
@@ -31,7 +31,6 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
-import java.util.Set;
/**
* Helper functions that implement often-used complex operations on file
@@ -630,25 +629,6 @@
*/
@ThreadSafe
public static boolean createDirectoryAndParents(Path dir) throws IOException {
- return createDirectoryAndParentsWithCache(null, dir);
- }
-
- /**
- * Attempts to create a directory with the name of the given path, creating ancestors as
- * necessary. Only creates directories or their parents if they are not contained in the set
- * {@code createdDirs} and instead assumes that they already exist. This saves a round-trip to the
- * kernel, but is only safe when no one deletes directories that have been created by this method.
- *
- * <p>Postcondition: completes normally iff {@code dir} denotes an existing directory (not
- * necessarily canonical); completes abruptly otherwise.
- *
- * @return true if the directory was successfully created anew, false if it already existed
- * (including the case where {@code dir} denotes a symlink to an existing directory)
- * @throws IOException if the directory could not be created
- */
- @ThreadSafe
- public static boolean createDirectoryAndParentsWithCache(Set<Path> createdDirs, Path dir)
- throws IOException {
// Optimised for minimal number of I/O calls.
// Don't attempt to create the root directory.
@@ -656,11 +636,6 @@
return false;
}
- // We already created that directory.
- if (createdDirs != null && createdDirs.contains(dir)) {
- return false;
- }
-
FileSystem filesystem = dir.getFileSystem();
if (filesystem instanceof UnionFileSystem) {
// If using UnionFS, make sure that we do not traverse filesystem boundaries when creating
@@ -670,23 +645,12 @@
}
try {
- boolean result = dir.createDirectory();
- if (createdDirs != null) {
- createdDirs.add(dir);
- }
- return result;
+ return dir.createDirectory();
} catch (IOException e) {
if (e.getMessage().endsWith(" (No such file or directory)")) { // ENOENT
- createDirectoryAndParentsWithCache(createdDirs, dir.getParentDirectory());
- boolean result = dir.createDirectory();
- if (createdDirs != null) {
- createdDirs.add(dir);
- }
- return result;
+ createDirectoryAndParents(dir.getParentDirectory());
+ return dir.createDirectory();
} else if (e.getMessage().endsWith(" (File exists)") && dir.isDirectory()) { // EEXIST
- if (createdDirs != null) {
- createdDirs.add(dir);
- }
return false;
} else {
throw e; // some other error (e.g. ENOTDIR, EACCES, etc.)
