From e798a520e8ae0d2f3baa5149390d3fb63c81c9c6 Mon Sep 17 00:00:00 2001 From: Brian Silverman Date: Mon, 15 Feb 2016 16:26:16 +0000 Subject: Reduce the number of stat calls when setting up a sandbox This improved performance for a (somewhat artificial) test which runs 100 genrules each with 3000 inputs by 25% on my laptop (2x hyperthreaded cores, SSD, ext4). Test code at . -- Change-Id: I7a7aaccdfbe2925c7e962c0192924ef1cf80b33a Reviewed-on: https://bazel-review.git.corp.google.com/#/c/2840/1..2 MOS_MIGRATED_REVID=114694334 --- .../build/lib/sandbox/LinuxSandboxedStrategy.java | 98 +++++++++++----------- 1 file changed, 50 insertions(+), 48 deletions(-) (limited to 'src/main/java/com') diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java index a6f73854a8..d67a6757dc 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java @@ -42,11 +42,13 @@ import com.google.devtools.build.lib.standalone.StandaloneSpawnStrategy; import com.google.devtools.build.lib.unix.NativePosixFiles; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.io.FileOutErr; +import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.SearchPath; +import com.google.devtools.build.lib.vfs.Symlinks; import java.io.File; import java.io.IOException; @@ -217,87 +219,84 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { private ImmutableMap getMounts(Spawn spawn, ActionExecutionContext executionContext) throws IOException, ExecException { + ImmutableMap.Builder result = new ImmutableMap.Builder<>(); + result.putAll(mountUsualUnixDirs()); MountMap mounts = new MountMap(); - mounts.putAll(mountUsualUnixDirs()); - mounts.putAll(withRecursedDirs(setupBlazeUtils())); - mounts.putAll(withRecursedDirs(mountRunfilesFromManifests(spawn))); - mounts.putAll(withRecursedDirs(mountRunfilesFromSuppliers(spawn))); - mounts.putAll(withRecursedDirs(mountFilesFromFilesetManifests(spawn, executionContext))); - mounts.putAll(withRecursedDirs(mountInputs(spawn, executionContext))); - mounts.putAll(withRecursedDirs(mountRunUnderCommand(spawn))); - return validateMounts(withResolvedSymlinks(mounts)); + mounts.putAll(setupBlazeUtils()); + mounts.putAll(mountRunfilesFromManifests(spawn)); + mounts.putAll(mountRunfilesFromSuppliers(spawn)); + mounts.putAll(mountFilesFromFilesetManifests(spawn, executionContext)); + mounts.putAll(mountInputs(spawn, executionContext)); + mounts.putAll(mountRunUnderCommand(spawn)); + result.putAll(finalizeMounts(mounts)); + return result.build(); } /** - * Validates all mounts against a set of criteria and throws an exception on error. + * Helper method of {@link #finalizeMounts}. This method handles adding a single path + * to the output map, including making sure it exists and adding the target of a + * symbolic link if necessary. * - * @return an ImmutableMap of all mounts. + * @param finalizedMounts the map to add the mapping(s) to + * @param target the key to add to the map + * @param source the value to add to the map + * @param stat information about source (passed in to avoid fetching it twice) */ - @VisibleForTesting - static ImmutableMap validateMounts(Map mounts) { - ImmutableMap.Builder validatedMounts = ImmutableMap.builder(); - for (Entry mount : mounts.entrySet()) { - Path target = mount.getKey(); - Path source = mount.getValue(); - - // The source must exist. - Preconditions.checkArgument(source.exists(), "%s does not exist", source.toString()); - - validatedMounts.put(target, source); + private static void finalizeMountPath( + MountMap finalizedMounts, Path target, Path source, FileStatus stat) throws IOException { + // The source must exist. + Preconditions.checkArgument(stat != null, "%s does not exist", source.toString()); + finalizedMounts.put(target, source); + + if (stat.isSymbolicLink()) { + Path symlinkTarget = source.resolveSymbolicLinks(); + Preconditions.checkArgument( + symlinkTarget.exists(), "%s does not exist", symlinkTarget.toString()); + finalizedMounts.put(symlinkTarget, symlinkTarget); } - return validatedMounts.build(); } /** + * Performs various checks on each mounted file which require stating each one. + * Contained in one function to allow minimizing the number of syscalls involved. + * * Checks for each mount if the source refers to a symbolic link and if yes, adds another mount * for the target of that symlink to ensure that it keeps working inside the sandbox. * - * @return a new mounts multimap with the added mounts. - */ - @VisibleForTesting - static MountMap withResolvedSymlinks(Map mounts) throws IOException { - MountMap fixedMounts = new MountMap(); - for (Entry mount : mounts.entrySet()) { - Path target = mount.getKey(); - Path source = mount.getValue(); - fixedMounts.put(target, source); - - if (source.isSymbolicLink()) { - Path symlinkTarget = source.resolveSymbolicLinks(); - fixedMounts.put(symlinkTarget, symlinkTarget); - } - } - return fixedMounts; - } - - /** * Checks for each mount if the source refers to a directory and if yes, replaces that mount with * mounts of all files inside that directory. * - * @return a new mounts multimap with the added mounts. + * Validates all mounts against a set of criteria and throws an exception on error. + * + * @return a new mounts multimap with all mounts and the added mounts. */ @VisibleForTesting - static MountMap withRecursedDirs(Map mounts) throws IOException { - MountMap fixedMounts = new MountMap(); + static MountMap finalizeMounts(Map mounts) throws IOException { + MountMap finalizedMounts = new MountMap(); for (Entry mount : mounts.entrySet()) { Path target = mount.getKey(); Path source = mount.getValue(); - if (source.isDirectory()) { + FileStatus stat = source.statNullable(Symlinks.NOFOLLOW); + + if (stat != null && stat.isDirectory()) { for (Path subSource : FileSystemUtils.traverseTree(source, Predicates.alwaysTrue())) { Path subTarget = target.getRelative(subSource.relativeTo(source)); - fixedMounts.put(subTarget, subSource); + finalizeMountPath( + finalizedMounts, subTarget, subSource, subSource.statNullable(Symlinks.NOFOLLOW)); } } else { - fixedMounts.put(target, source); + finalizeMountPath(finalizedMounts, target, source, stat); } } - return fixedMounts; + return finalizedMounts; } /** * Mount a certain set of unix directories to make the usual tools and libraries available to the * spawn that runs. + * + * Throws an exception if any of them do not exist. */ private MountMap mountUsualUnixDirs() throws IOException { MountMap mounts = new MountMap(); @@ -317,6 +316,9 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { mounts.put(usrDir, usrDir); } } + for (Path path : mounts.values()) { + Preconditions.checkArgument(path.exists(), "%s does not exist", path.toString()); + } return mounts; } -- cgit v1.2.3