Skip to content

Commit

Permalink
travis: Parallelize tests on Android
Browse files Browse the repository at this point in the history
Currently our slowest test suite on android, run-pass, takes over 5 times longer
than the x86_64 component (~400 -> ~2200s). Typically QEMU emulation does indeed
add overhead, but not 5x for this kind of workload. One of the slowest parts of
the Android process is that *compilation* happens serially. Tests themselves
need to run single-threaded on the emulator (due to how the test harness works)
and this forces the compiles themselves to be single threaded.

Now Travis gives us more than one core per machine, so it'd be much better if we
could take advantage of them! The emulator itself is still fundamentally
single-threaded, but we should see a nice speedup by sending binaries for it to
run much more quickly.

It turns out that we've already got all the tools to do this in-tree. The
qemu-test-{server,client} that are in use for the ARM Linux testing are a
perfect match for the Android emulator. This commit migrates the custom adb
management code in compiletest/rustbuild to the same qemu-test-{server,client}
implementation that ARM Linux uses.

This allows us to lift the parallelism restriction on the compiletest test
suites, namely run-pass. Consequently although we'll still basically run the
tests themselves in single threaded mode we'll be able to compile all of them in
parallel, keeping the pipeline much more full and using more cores for the work
at hand. Additionally the architecture here should be a bit speedier as it
should have less overhead than adb which is a whole new process on both the host
and the emulator!

Locally on an 8 core machine I've seen the run-pass test suite speed up from
taking nearly an hour to only taking 6 minutes. I don't think we'll see quite a
drastic speedup on Travis but I'm hoping this change can place the Android tests
well below 2 hours instead of just above 2 hours.

Because the client/server here are now repurposed for more than just QEMU,
they've been renamed to `remote-test-{server,client}`.

Note that this PR does not currently modify how debuginfo tests are executed on
Android. While parallelizable it wouldn't be quite as easy, so that's left to
another day. Thankfully that test suite is much smaller than the run-pass test
suite.

As a final fix I discovered that the ARM and Android test suites were actually
running all library unit tests (e.g. stdtest, coretest, etc) twice. I've
corrected that to only run tests once which should also give a nice boost in
overall cycle time here.
  • Loading branch information
alexcrichton committed Apr 28, 2017
1 parent 70baf4f commit 7bc2cbf
Show file tree
Hide file tree
Showing 14 changed files with 243 additions and 467 deletions.
16 changes: 8 additions & 8 deletions src/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Cargo.toml
Expand Up @@ -11,8 +11,8 @@ members = [
"tools/rustbook",
"tools/tidy",
"tools/build-manifest",
"tools/qemu-test-client",
"tools/qemu-test-server",
"tools/remote-test-client",
"tools/remote-test-server",
]

# Curiously, compiletest will segfault if compiled with opt-level=3 on 64-bit
Expand Down
140 changes: 27 additions & 113 deletions src/bootstrap/check.rs
Expand Up @@ -28,7 +28,7 @@ use {Build, Compiler, Mode};
use dist;
use util::{self, dylib_path, dylib_path_var, exe};

const ADB_TEST_DIR: &'static str = "/data/tmp";
const ADB_TEST_DIR: &'static str = "/data/tmp/work";

/// The two modes of the test runner; tests or benchmarks.
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -243,10 +243,10 @@ pub fn compiletest(build: &Build,
.arg("--llvm-cxxflags").arg("");
}

if build.qemu_rootfs(target).is_some() {
cmd.arg("--qemu-test-client")
if build.remote_tested(target) {
cmd.arg("--remote-test-client")
.arg(build.tool(&Compiler::new(0, &build.config.build),
"qemu-test-client"));
"remote-test-client"));
}

// Running a C compiler on MSVC requires a few env vars to be set, to be
Expand Down Expand Up @@ -445,9 +445,7 @@ pub fn krate(build: &Build,
dylib_path.insert(0, build.sysroot_libdir(&compiler, target));
cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());

if target.contains("android") ||
target.contains("emscripten") ||
build.qemu_rootfs(target).is_some() {
if target.contains("emscripten") || build.remote_tested(target) {
cargo.arg("--no-run");
}

Expand All @@ -459,75 +457,24 @@ pub fn krate(build: &Build,

let _time = util::timeit();

if target.contains("android") {
build.run(&mut cargo);
krate_android(build, &compiler, target, mode);
} else if target.contains("emscripten") {
if target.contains("emscripten") {
build.run(&mut cargo);
krate_emscripten(build, &compiler, target, mode);
} else if build.qemu_rootfs(target).is_some() {
} else if build.remote_tested(target) {
build.run(&mut cargo);
krate_qemu(build, &compiler, target, mode);
krate_remote(build, &compiler, target, mode);
} else {
cargo.args(&build.flags.cmd.test_args());
build.run(&mut cargo);
}
}

fn krate_android(build: &Build,
compiler: &Compiler,
target: &str,
mode: Mode) {
let mut tests = Vec::new();
let out_dir = build.cargo_out(compiler, mode, target);
find_tests(&out_dir, target, &mut tests);
find_tests(&out_dir.join("deps"), target, &mut tests);

for test in tests {
build.run(Command::new("adb").arg("push").arg(&test).arg(ADB_TEST_DIR));

let test_file_name = test.file_name().unwrap().to_string_lossy();
let log = format!("{}/check-stage{}-T-{}-H-{}-{}.log",
ADB_TEST_DIR,
compiler.stage,
target,
compiler.host,
test_file_name);
let quiet = if build.config.quiet_tests { "--quiet" } else { "" };
let program = format!("(cd {dir}; \
LD_LIBRARY_PATH=./{target} ./{test} \
--logfile {log} \
{quiet} \
{args})",
dir = ADB_TEST_DIR,
target = target,
test = test_file_name,
log = log,
quiet = quiet,
args = build.flags.cmd.test_args().join(" "));

let output = output(Command::new("adb").arg("shell").arg(&program));
println!("{}", output);

t!(fs::create_dir_all(build.out.join("tmp")));
build.run(Command::new("adb")
.arg("pull")
.arg(&log)
.arg(build.out.join("tmp")));
build.run(Command::new("adb").arg("shell").arg("rm").arg(&log));
if !output.contains("result: ok") {
panic!("some tests failed");
}
}
}

fn krate_emscripten(build: &Build,
compiler: &Compiler,
target: &str,
mode: Mode) {
let mut tests = Vec::new();
let out_dir = build.cargo_out(compiler, mode, target);
find_tests(&out_dir, target, &mut tests);
find_tests(&out_dir.join("deps"), target, &mut tests);

for test in tests {
Expand All @@ -543,17 +490,16 @@ fn krate_emscripten(build: &Build,
}
}

fn krate_qemu(build: &Build,
compiler: &Compiler,
target: &str,
mode: Mode) {
fn krate_remote(build: &Build,
compiler: &Compiler,
target: &str,
mode: Mode) {
let mut tests = Vec::new();
let out_dir = build.cargo_out(compiler, mode, target);
find_tests(&out_dir, target, &mut tests);
find_tests(&out_dir.join("deps"), target, &mut tests);

let tool = build.tool(&Compiler::new(0, &build.config.build),
"qemu-test-client");
"remote-test-client");
for test in tests {
let mut cmd = Command::new(&tool);
cmd.arg("run")
Expand All @@ -566,7 +512,6 @@ fn krate_qemu(build: &Build,
}
}


fn find_tests(dir: &Path,
target: &str,
dst: &mut Vec<PathBuf>) {
Expand All @@ -585,59 +530,28 @@ fn find_tests(dir: &Path,
}

pub fn emulator_copy_libs(build: &Build, compiler: &Compiler, target: &str) {
if target.contains("android") {
android_copy_libs(build, compiler, target)
} else if let Some(s) = build.qemu_rootfs(target) {
qemu_copy_libs(build, compiler, target, s)
}
}

fn android_copy_libs(build: &Build, compiler: &Compiler, target: &str) {
println!("Android copy libs to emulator ({})", target);
build.run(Command::new("adb").arg("wait-for-device"));
build.run(Command::new("adb").arg("remount"));
build.run(Command::new("adb").args(&["shell", "rm", "-r", ADB_TEST_DIR]));
build.run(Command::new("adb").args(&["shell", "mkdir", ADB_TEST_DIR]));
build.run(Command::new("adb")
.arg("push")
.arg(build.src.join("src/etc/adb_run_wrapper.sh"))
.arg(ADB_TEST_DIR));

let target_dir = format!("{}/{}", ADB_TEST_DIR, target);
build.run(Command::new("adb").args(&["shell", "mkdir", &target_dir]));

for f in t!(build.sysroot_libdir(compiler, target).read_dir()) {
let f = t!(f);
let name = f.file_name().into_string().unwrap();
if util::is_dylib(&name) {
build.run(Command::new("adb")
.arg("push")
.arg(f.path())
.arg(&target_dir));
}
if !build.remote_tested(target) {
return
}
}

fn qemu_copy_libs(build: &Build,
compiler: &Compiler,
target: &str,
rootfs: &Path) {
println!("QEMU copy libs to emulator ({})", target);
assert!(target.starts_with("arm"), "only works with arm for now");
println!("REMOTE copy libs to emulator ({})", target);
t!(fs::create_dir_all(build.out.join("tmp")));

// Copy our freshly compiled test server over to the rootfs
let server = build.cargo_out(compiler, Mode::Tool, target)
.join(exe("qemu-test-server", target));
t!(fs::copy(&server, rootfs.join("testd")));
.join(exe("remote-test-server", target));

// Spawn the emulator and wait for it to come online
let tool = build.tool(&Compiler::new(0, &build.config.build),
"qemu-test-client");
build.run(Command::new(&tool)
.arg("spawn-emulator")
.arg(rootfs)
.arg(build.out.join("tmp")));
"remote-test-client");
let mut cmd = Command::new(&tool);
cmd.arg("spawn-emulator")
.arg(target)
.arg(&server)
.arg(build.out.join("tmp"));
if let Some(rootfs) = build.qemu_rootfs(target) {
cmd.arg(rootfs);
}
build.run(&mut cmd);

// Push all our dylibs to the emulator
for f in t!(build.sysroot_libdir(compiler, target).read_dir()) {
Expand Down
6 changes: 6 additions & 0 deletions src/bootstrap/lib.rs
Expand Up @@ -945,6 +945,12 @@ impl Build {
.map(|p| &**p)
}

/// Returns whether the target will be tested using the `remote-test-client`
/// and `remote-test-server` binaries.
fn remote_tested(&self, target: &str) -> bool {
self.qemu_rootfs(target).is_some() || target.contains("android")
}

/// Returns the root of the "rootfs" image that this target will be using,
/// if one was configured.
///
Expand Down
16 changes: 8 additions & 8 deletions src/bootstrap/step.rs
Expand Up @@ -513,15 +513,15 @@ pub fn build_rules<'a>(build: &'a Build) -> Rules {
rules.test("emulator-copy-libs", "path/to/nowhere")
.dep(|s| s.name("libtest"))
.dep(move |s| {
if build.qemu_rootfs(s.target).is_some() {
s.name("tool-qemu-test-client").target(s.host).stage(0)
if build.remote_tested(s.target) {
s.name("tool-remote-test-client").target(s.host).stage(0)
} else {
Step::noop()
}
})
.dep(move |s| {
if build.qemu_rootfs(s.target).is_some() {
s.name("tool-qemu-test-server")
if build.remote_tested(s.target) {
s.name("tool-remote-test-server")
} else {
Step::noop()
}
Expand Down Expand Up @@ -566,14 +566,14 @@ pub fn build_rules<'a>(build: &'a Build) -> Rules {
.dep(|s| s.name("maybe-clean-tools"))
.dep(|s| s.name("libstd-tool"))
.run(move |s| compile::tool(build, s.stage, s.target, "build-manifest"));
rules.build("tool-qemu-test-server", "src/tools/qemu-test-server")
rules.build("tool-remote-test-server", "src/tools/remote-test-server")
.dep(|s| s.name("maybe-clean-tools"))
.dep(|s| s.name("libstd-tool"))
.run(move |s| compile::tool(build, s.stage, s.target, "qemu-test-server"));
rules.build("tool-qemu-test-client", "src/tools/qemu-test-client")
.run(move |s| compile::tool(build, s.stage, s.target, "remote-test-server"));
rules.build("tool-remote-test-client", "src/tools/remote-test-client")
.dep(|s| s.name("maybe-clean-tools"))
.dep(|s| s.name("libstd-tool"))
.run(move |s| compile::tool(build, s.stage, s.target, "qemu-test-client"));
.run(move |s| compile::tool(build, s.stage, s.target, "remote-test-client"));
rules.build("tool-cargo", "cargo")
.dep(|s| s.name("maybe-clean-tools"))
.dep(|s| s.name("libstd-tool"))
Expand Down
35 changes: 0 additions & 35 deletions src/etc/adb_run_wrapper.sh

This file was deleted.

13 changes: 12 additions & 1 deletion src/test/run-pass/vector-sort-panic-safe.rs
Expand Up @@ -13,9 +13,11 @@
#![feature(rand)]
#![feature(const_fn)]

use std::sync::atomic::{AtomicUsize, Ordering};
use std::__rand::{thread_rng, Rng};
use std::panic;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::thread;
use std::cell::Cell;

const MAX_LEN: usize = 80;

Expand Down Expand Up @@ -76,6 +78,7 @@ fn test(input: &[DropCounter]) {
let mut panic_countdown = panic_countdown;
v.sort_by(|a, b| {
if panic_countdown == 0 {
SILENCE_PANIC.with(|s| s.set(true));
panic!();
}
panic_countdown -= 1;
Expand All @@ -94,7 +97,15 @@ fn test(input: &[DropCounter]) {
}
}

thread_local!(static SILENCE_PANIC: Cell<bool> = Cell::new(false));

fn main() {
let prev = panic::take_hook();
panic::set_hook(Box::new(move |info| {
if !SILENCE_PANIC.with(|s| s.get()) {
prev(info);
}
}));
for len in (1..20).chain(70..MAX_LEN) {
// Test on a random array.
let mut rng = thread_rng();
Expand Down
4 changes: 2 additions & 2 deletions src/tools/compiletest/src/common.rs
Expand Up @@ -185,8 +185,8 @@ pub struct Config {
// Print one character per test instead of one line
pub quiet: bool,

// where to find the qemu test client process, if we're using it
pub qemu_test_client: Option<PathBuf>,
// where to find the remote test client process, if we're using it
pub remote_test_client: Option<PathBuf>,

// Configuration for various run-make tests frobbing things like C compilers
// or querying about various LLVM component information.
Expand Down

0 comments on commit 7bc2cbf

Please sign in to comment.