Skip to content

Commit

Permalink
Auto merge of #41575 - alexcrichton:android-qemu-server, r=TimNN
Browse files Browse the repository at this point in the history
travis: Parallelize tests on Android

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 toos 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 hopefully 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 5 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. Thankfull that test suite is much smaller than the run-pass test
suite.
  • Loading branch information
bors committed Apr 28, 2017
2 parents 2971d49 + 7bc2cbf commit ad1461e
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit ad1461e

Please sign in to comment.