Skip to content

Commit

Permalink
Make sandbox symlink forest deletions asynchronous.
Browse files Browse the repository at this point in the history
Each sandbox action runs within a symlink forest that exists in a separate
subtree because we use a unique identifier for those subtrees.  Therefore
it is unnecessary to delete those trees in the critical path.

Tree deletions can be very expensive, especially on macOS, so make them
asynchronous if --experimental_sandbox_async_tree_delete_idle_threads is
given.  When this flag is not zero, Bazel will schedule all deletions on
a separate low-priority thread while the build is running, and will then
use the requested number of threads once the build is done to quickly
catch up with an still-ongoing deletions.

For a large iOS build, this cuts down clean build times with sandboxed
enabled significantly.  Helps more on machines with more cores:

* On a Mac Pro 2013, the improvement is almost 20%:

  standalone:      mean 2746.33, median 2736.00, stddev 33.07
  sandboxed-async: mean 4394.67, median 4393.00, stddev 33.09
  sandboxed-sync:  mean 5284.33, median 5288.00, stddev 20.17

* On a MacBook Pro 2015, we see a more modest 10% improvement:

  standalone:      mean 3418.33, median 3422.00, stddev 7.41
  sandboxed-async: mean 5090.00, median 5086.00, stddev 40.92
  sandboxed-sync:  mean 5694.67, median 5700.00, stddev 37.75

Partially addresses #7527.

RELNOTES: None.
PiperOrigin-RevId: 243805556
  • Loading branch information
jmmv authored and copybara-github committed Apr 16, 2019
1 parent 5074514 commit 16af94c
Show file tree
Hide file tree
Showing 22 changed files with 415 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
*
* @param sandboxBase path to the base of the sandbox tree where the spawn runner may have created
* entries
* @param treeDeleter scheduler for tree deletions
* @throws IOException if there are problems deleting the entries
*/
default void cleanupSandboxBase(Path sandboxBase) throws IOException {}
default void cleanupSandboxBase(Path sandboxBase, TreeDeleter treeDeleter) throws IOException {}
}
49 changes: 49 additions & 0 deletions src/main/java/com/google/devtools/build/lib/exec/TreeDeleter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2019 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.devtools.build.lib.exec;

import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;

/** Interface to execute tree deletions under different scheduling policies. */
public interface TreeDeleter {

/**
* Deletes a tree.
*
* <p>Note that depending on the scheduling policy implemented by this tree deleter, errors may
* not be reported even if they happen. For example, if deletions are asynchronous, there is no
* way to capture their errors.
*
* @param path the tree to be deleted
* @throws IOException if there are problems deleting the tree
*/
void deleteTree(Path path) throws IOException;

/**
* Deletes the contents of a tree, but not the top-level directory.
*
* <p>Note that depending on the scheduling policy implemented by this tree deleter, errors may
* not be reported even if they happen. For example, if deletions are asynchronous, there is no
* way to capture their errors.
*
* @param path the tree to be deleted
* @throws IOException if there are problems deleting the tree
*/
void deleteTreesBelow(Path path) throws IOException;

/** Shuts down the tree deleter and cleans up pending operations, if any. */
void shutdown();
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.common.base.Preconditions;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.FileSystemUtils.MoveResult;
Expand Down Expand Up @@ -47,6 +48,7 @@ public abstract class AbstractContainerizingSandboxedSpawn implements SandboxedS
private final Map<PathFragment, Path> inputs;
private final SandboxOutputs outputs;
private final Set<Path> writableDirs;
private final TreeDeleter treeDeleter;

public AbstractContainerizingSandboxedSpawn(
Path sandboxPath,
Expand All @@ -55,14 +57,16 @@ public AbstractContainerizingSandboxedSpawn(
Map<String, String> environment,
Map<PathFragment, Path> inputs,
SandboxOutputs outputs,
Set<Path> writableDirs) {
Set<Path> writableDirs,
TreeDeleter treeDeleter) {
this.sandboxPath = sandboxPath;
this.sandboxExecRoot = sandboxExecRoot;
this.arguments = arguments;
this.environment = environment;
this.inputs = inputs;
this.outputs = outputs;
this.writableDirs = writableDirs;
this.treeDeleter = treeDeleter;
}

@Override
Expand Down Expand Up @@ -189,7 +193,7 @@ public void copyOutputs(Path execRoot) throws IOException {
@Override
public void delete() {
try {
sandboxPath.deleteTree();
treeDeleter.deleteTree(sandboxPath);
} catch (IOException e) {
// This usually means that the Spawn itself exited, but still has children running that
// we couldn't wait for, which now block deletion of the sandbox directory. On Linux this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.exec.BinTools;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.SpawnRunner;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.shell.AbnormalTerminationException;
import com.google.devtools.build.lib.shell.Command;
Expand Down Expand Up @@ -290,11 +291,11 @@ protected SandboxOptions getSandboxOptions() {
}

@Override
public void cleanupSandboxBase(Path sandboxBase) throws IOException {
public void cleanupSandboxBase(Path sandboxBase, TreeDeleter treeDeleter) throws IOException {
Path root = sandboxBase.getChild(getName());
if (root.exists()) {
for (Path child : root.getDirectoryEntries()) {
child.deleteTree();
treeDeleter.deleteTree(child);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Copyright 2019 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.devtools.build.lib.sandbox;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;

import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;
import javax.annotation.Nullable;

/**
* Executes file system tree deletions asynchronously.
*
* <p>The number of threads used to process the backlog of tree deletions can be configured at any
* time via {@link #setThreads(int)}. While a build is running, this number should be low to not use
* precious resources that could otherwise be used for the build itself. But when the build is
* finished, this number should be raised to quickly go through any pending deletions.
*/
class AsynchronousTreeDeleter implements TreeDeleter {

private static final Logger logger = Logger.getLogger(TreeDeleter.class.getName());

/** Thread pool used to execute asynchronous tree deletions; null in synchronous mode. */
@Nullable private ThreadPoolExecutor service;

/** Constructs a new asynchronous tree deleter backed by just one thread. */
AsynchronousTreeDeleter() {
logger.info("Starting async tree deletion pool with 1 thread");

ThreadFactory threadFactory =
new ThreadFactoryBuilder()
.setNameFormat("tree-deleter")
.setDaemon(true)
.setPriority(Thread.MIN_PRIORITY)
.build();

service =
new ThreadPoolExecutor(
1, 1, 0L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), threadFactory);
}

/**
* Resizes the thread pool to the given number of threads.
*
* <p>If the pool of active threads is larger than the requested number of threads, the resize
* will progressively happen as those active threads become inactive. If the requested size is
* zero, this will wait for all pending deletions to complete.
*
* @param threads desired number of threads, or 0 to go back to synchronous deletion
*/
void setThreads(int threads) {
checkState(threads > 0, "Use SynchronousTreeDeleter if no async behavior is desired");
logger.info("Resizing async tree deletion pool to " + threads + " threads");
checkNotNull(service, "Cannot call setThreads after shutdown").setMaximumPoolSize(threads);
}

@Override
public void deleteTree(Path path) {
checkNotNull(service, "Cannot call deleteTree after shutdown")
.execute(
() -> {
try {
path.deleteTree();
} catch (IOException e) {
logger.warning("Failed to delete tree " + path + " asynchronously: " + e);
}
});
}

@Override
public void deleteTreesBelow(Path path) {
checkNotNull(service, "Cannot call deleteTree after shutdown")
.execute(
() -> {
try {
path.deleteTreesBelow();
} catch (IOException e) {
logger.warning("Failed to delete contents of " + path + " asynchronously: " + e);
}
});
}

@Override
public void shutdown() {
if (service != null) {
logger.info("Finishing " + service.getTaskCount() + " pending async tree deletions");
service.shutdown();
service = null;
}
}
}
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/sandbox/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ java_library(
"//src/main/java/com/google/devtools/build/lib:io",
"//src/main/java/com/google/devtools/build/lib:out-err",
"//src/main/java/com/google/devtools/build/lib:process_util",
"//src/main/java/com/google/devtools/build/lib:resource-converter",
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
"//src/main/java/com/google/devtools/build/lib/exec/apple",
"//src/main/java/com/google/devtools/build/lib/exec/local",
"//src/main/java/com/google/devtools/build/lib/exec/local:options",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.google.devtools.build.lib.sandbox;

import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
Expand All @@ -37,8 +38,17 @@ public CopyingSandboxedSpawn(
Map<String, String> environment,
Map<PathFragment, Path> inputs,
SandboxOutputs outputs,
Set<Path> writableDirs) {
super(sandboxPath, sandboxExecRoot, arguments, environment, inputs, outputs, writableDirs);
Set<Path> writableDirs,
TreeDeleter treeDeleter) {
super(
sandboxPath,
sandboxExecRoot,
arguments,
environment,
inputs,
outputs,
writableDirs,
treeDeleter);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.exec.apple.XcodeLocalEnvProvider;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
Expand Down Expand Up @@ -113,6 +114,7 @@ private static boolean computeIsSupported() {
private final Duration timeoutKillDelay;
private final @Nullable SandboxfsProcess sandboxfsProcess;
private final boolean sandboxfsMapSymlinkTargets;
private final TreeDeleter treeDeleter;

/**
* The set of directories that always should be writable, independent of the Spawn itself.
Expand All @@ -138,7 +140,8 @@ private static boolean computeIsSupported() {
Path sandboxBase,
Duration timeoutKillDelay,
@Nullable SandboxfsProcess sandboxfsProcess,
boolean sandboxfsMapSymlinkTargets)
boolean sandboxfsMapSymlinkTargets,
TreeDeleter treeDeleter)
throws IOException {
super(cmdEnv);
this.execRoot = cmdEnv.getExecRoot();
Expand All @@ -150,6 +153,7 @@ private static boolean computeIsSupported() {
this.timeoutKillDelay = timeoutKillDelay;
this.sandboxfsProcess = sandboxfsProcess;
this.sandboxfsMapSymlinkTargets = sandboxfsMapSymlinkTargets;
this.treeDeleter = treeDeleter;
}

private static void addPathToSetIfExists(FileSystem fs, Set<Path> paths, String path)
Expand Down Expand Up @@ -282,7 +286,8 @@ protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context)
inputs,
outputs,
ImmutableSet.of(),
sandboxfsMapSymlinkTargets) {
sandboxfsMapSymlinkTargets,
treeDeleter) {
@Override
public void createFileSystem() throws IOException {
super.createFileSystem();
Expand All @@ -303,7 +308,8 @@ public void createFileSystem() throws IOException {
environment,
inputs,
outputs,
writableDirs) {
writableDirs,
treeDeleter) {
@Override
public void createFileSystem() throws IOException {
super.createFileSystem();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.PosixLocalEnvProvider;
import com.google.devtools.build.lib.runtime.CommandCompleteEvent;
Expand Down Expand Up @@ -146,6 +147,7 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)
private final String commandId;
private final Reporter reporter;
private final boolean useCustomizedImages;
private final TreeDeleter treeDeleter;
private final int uid;
private final int gid;
private final List<UUID> containersToCleanup;
Expand All @@ -160,14 +162,16 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)
* @param defaultImage the Docker image to use if the platform doesn't specify one
* @param timeoutKillDelay an additional grace period before killing timing out commands
* @param useCustomizedImages whether to use customized images for execution
* @param treeDeleter scheduler for tree deletions
*/
DockerSandboxedSpawnRunner(
CommandEnvironment cmdEnv,
Path dockerClient,
Path sandboxBase,
String defaultImage,
Duration timeoutKillDelay,
boolean useCustomizedImages) {
boolean useCustomizedImages,
TreeDeleter treeDeleter) {
super(cmdEnv);
this.execRoot = cmdEnv.getExecRoot();
this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions());
Expand All @@ -180,6 +184,7 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)
this.commandId = cmdEnv.getCommandId().toString();
this.reporter = cmdEnv.getReporter();
this.useCustomizedImages = useCustomizedImages;
this.treeDeleter = treeDeleter;
this.cmdEnv = cmdEnv;
if (OS.getCurrent() == OS.LINUX) {
this.uid = ProcessUtils.getuid();
Expand Down Expand Up @@ -270,7 +275,8 @@ protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context)
execRoot,
getSandboxOptions().symlinkedSandboxExpandsTreeArtifactsInRunfilesTree),
outputs,
ImmutableSet.of());
ImmutableSet.of(),
treeDeleter);

try {
return runSpawn(spawn, sandbox, context, execRoot, timeout, null);
Expand Down
Loading

0 comments on commit 16af94c

Please sign in to comment.