From 10fef22e624824a7035c5b37e65156abf9e6d740 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 12 Apr 2019 09:06:54 -0700 Subject: [PATCH] Add support for automatically executing `wasm-opt` This commit adds support for automatically executing the `wasm-opt` binary from the [Binaryen project][binaryen]. By default `wasm-pack` will now, in release and profiling modes, execute `wasm-opt -O` over the final binary. The goal here is to enable optimizations that further reduce binary size or improve runtime. In the long run it's expected that `wasm-opt`'s optimizations may mostly make their way into LLVM, but it's empirically true today that `wasm-opt` plus LLVM is the best combination for size and speed today. A configuration section for `wasm-opt` has been added as [previously proposed][fitzgen], namely: ```toml [package.metadata.wasm-pack.profile.release] wasm-opt = ['-Os'] ``` The `wasm-opt` binary is downloaded from Binaryen's [own releases](https://github.com/webassembly/binaryen/releases). They're available for the same platforms that we download predownloaded binaries for `wasm-bindgen` on. We'll also opportunistically use `wasm-opt` in `PATH` if it's available. If we're untable to run `wasm-opt`, though, a warning diagnostic is printed informing such. Closes #159 [binaryen]: https://github.com/webassembly/binaryen [fitzgen]: https://github.com/rustwasm/wasm-pack/issues/159#issuecomment-454888890 --- docs/src/cargo-toml-configuration.md | 17 +++ src/child.rs | 10 +- src/command/build.rs | 43 +++++- src/lib.rs | 1 + src/manifest/mod.rs | 31 +++++ src/opt.rs | 108 +++++++++++++++ tests/all/main.rs | 1 + tests/all/opt.rs | 195 +++++++++++++++++++++++++++ tests/all/utils/fixture.rs | 9 ++ 9 files changed, 404 insertions(+), 11 deletions(-) create mode 100644 src/opt.rs create mode 100644 tests/all/opt.rs diff --git a/docs/src/cargo-toml-configuration.md b/docs/src/cargo-toml-configuration.md index 5515ecb08..38414bb40 100644 --- a/docs/src/cargo-toml-configuration.md +++ b/docs/src/cargo-toml-configuration.md @@ -9,6 +9,17 @@ the `--dev`, `--profiling`, and `--release` flags passed to `wasm-pack build`. The available configuration options and their default values are shown below: ```toml +[package.metadata.wasm-pack.profile.dev] +# Should `wasm-opt` be used to further optimize the wasm binary generated after +# the Rust compiler has finished? Using `wasm-opt` can often further decrease +# binary size or do clever tricks that haven't made their way into LLVM yet. +# +# Configuration can be set to `false` if you want to disable it (as is the +# default for the dev profile), or it can be an array of strings which are +# explicit arguments to pass to `wasm-opt`. For example `['-Os']` would optimize +# for size while `['-O4']` would execute very expensive optimizations passes +wasm-opt = false + [package.metadata.wasm-pack.profile.dev.wasm-bindgen] # Should we enable wasm-bindgen's debug assertions in its generated JS glue? debug-js-glue = true @@ -17,11 +28,17 @@ demangle-name-section = true # Should we emit the DWARF debug info custom sections? dwarf-debug-info = false +[package.metadata.wasm-pack.profile.profiling] +wasm-opt = ['-O'] + [package.metadata.wasm-pack.profile.profiling.wasm-bindgen] debug-js-glue = false demangle-name-section = true dwarf-debug-info = false +[package.metadata.wasm-pack.profile.release] +wasm-opt = ['-O'] + [package.metadata.wasm-pack.profile.release.wasm-bindgen] debug-js-glue = false demangle-name-section = true diff --git a/src/child.rs b/src/child.rs index a5cfce8a0..a85c2374f 100644 --- a/src/child.rs +++ b/src/child.rs @@ -33,9 +33,10 @@ pub fn run(mut command: Command, command_name: &str) -> Result<(), Error> { Ok(()) } else { bail!( - "failed to execute `{}`: exited with {}", + "failed to execute `{}`: exited with {}\n full command: {:?}", command_name, - status + status, + command, ) } } @@ -53,9 +54,10 @@ pub fn run_capture_stdout(mut command: Command, command_name: &str) -> Result bool { + match self { + BuildMode::Normal => true, + BuildMode::Force => true, + BuildMode::Noinstall => false, + } + } +} + impl Default for BuildMode { fn default() -> BuildMode { BuildMode::Normal @@ -284,6 +295,7 @@ impl Build { step_copy_license, step_install_wasm_bindgen, step_run_wasm_bindgen, + step_run_wasm_opt, step_create_json, ]); steps @@ -366,13 +378,11 @@ impl Build { let lockfile = Lockfile::new(&self.crate_data)?; let bindgen_version = lockfile.require_wasm_bindgen()?; info!("Installing wasm-bindgen-cli..."); - let install_permitted = match self.mode { - BuildMode::Normal => true, - BuildMode::Force => true, - BuildMode::Noinstall => false, - }; - let bindgen = - bindgen::install_wasm_bindgen(&self.cache, &bindgen_version, install_permitted)?; + let bindgen = bindgen::install_wasm_bindgen( + &self.cache, + &bindgen_version, + self.mode.install_permitted(), + )?; self.bindgen = Some(bindgen); info!("Installing wasm-bindgen-cli was successful."); Ok(()) @@ -392,4 +402,23 @@ impl Build { info!("wasm bindings were built at {:#?}.", &self.out_dir); Ok(()) } + + fn step_run_wasm_opt(&mut self) -> Result<(), Error> { + let args = match self + .crate_data + .configured_profile(self.profile) + .wasm_opt_args() + { + Some(args) => args, + None => return Ok(()), + }; + info!("executing wasm-opt with {:?}", args); + opt::run( + &self.cache, + &self.out_dir, + &args, + self.mode.install_permitted(), + )?; + Ok(()) + } } diff --git a/src/lib.rs b/src/lib.rs index e8b7ceb68..a9d3c1cf2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,6 +33,7 @@ pub mod license; pub mod lockfile; pub mod manifest; pub mod npm; +pub mod opt; pub mod progressbar; pub mod readme; pub mod target; diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index 25047d62c..42d1de60c 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -99,6 +99,8 @@ impl Default for CargoWasmPackProfiles { pub struct CargoWasmPackProfile { #[serde(default, rename = "wasm-bindgen")] wasm_bindgen: CargoWasmPackProfileWasmBindgen, + #[serde(default, rename = "wasm-opt")] + wasm_opt: Option, } #[derive(Default, Deserialize)] @@ -113,6 +115,19 @@ struct CargoWasmPackProfileWasmBindgen { dwarf_debug_info: Option, } +#[derive(Clone, Deserialize)] +#[serde(untagged)] +enum CargoWasmPackProfileWasmOpt { + Enabled(bool), + ExplicitArgs(Vec), +} + +impl Default for CargoWasmPackProfileWasmOpt { + fn default() -> Self { + CargoWasmPackProfileWasmOpt::Enabled(false) + } +} + impl CargoWasmPackProfile { fn default_dev() -> Self { CargoWasmPackProfile { @@ -121,6 +136,7 @@ impl CargoWasmPackProfile { demangle_name_section: Some(true), dwarf_debug_info: Some(false), }, + wasm_opt: None, } } @@ -131,6 +147,7 @@ impl CargoWasmPackProfile { demangle_name_section: Some(true), dwarf_debug_info: Some(false), }, + wasm_opt: Some(CargoWasmPackProfileWasmOpt::Enabled(true)), } } @@ -141,6 +158,7 @@ impl CargoWasmPackProfile { demangle_name_section: Some(true), dwarf_debug_info: Some(false), }, + wasm_opt: Some(CargoWasmPackProfileWasmOpt::Enabled(true)), } } @@ -180,6 +198,10 @@ impl CargoWasmPackProfile { d!(wasm_bindgen.debug_js_glue); d!(wasm_bindgen.demangle_name_section); d!(wasm_bindgen.dwarf_debug_info); + + if self.wasm_opt.is_none() { + self.wasm_opt = defaults.wasm_opt.clone(); + } } /// Get this profile's configured `[wasm-bindgen.debug-js-glue]` value. @@ -196,6 +218,15 @@ impl CargoWasmPackProfile { pub fn wasm_bindgen_dwarf_debug_info(&self) -> bool { self.wasm_bindgen.dwarf_debug_info.unwrap() } + + /// Get this profile's configured arguments for `wasm-opt`, if enabled. + pub fn wasm_opt_args(&self) -> Option> { + match self.wasm_opt.as_ref()? { + CargoWasmPackProfileWasmOpt::Enabled(false) => None, + CargoWasmPackProfileWasmOpt::Enabled(true) => Some(vec!["-O".to_string()]), + CargoWasmPackProfileWasmOpt::ExplicitArgs(s) => Some(s.clone()), + } + } } struct NpmData { diff --git a/src/opt.rs b/src/opt.rs new file mode 100644 index 000000000..c76818d21 --- /dev/null +++ b/src/opt.rs @@ -0,0 +1,108 @@ +//! Support for downloading and executing `wasm-opt` + +use crate::child; +use crate::emoji; +use crate::target; +use crate::PBAR; +use binary_install::Cache; +use log::debug; +use std::path::{Path, PathBuf}; +use std::process::Command; + +/// Execute `wasm-opt` over wasm binaries found in `out_dir`, downloading if +/// necessary into `cache`. Passes `args` to each invocation of `wasm-opt`. +pub fn run( + cache: &Cache, + out_dir: &Path, + args: &[String], + install_permitted: bool, +) -> Result<(), failure::Error> { + let wasm_opt = match find_wasm_opt(cache, install_permitted)? { + WasmOpt::Found(path) => path, + WasmOpt::CannotInstall => { + PBAR.info("Skipping wasm-opt as no downloading was requested"); + return Ok(()); + } + WasmOpt::PlatformNotSupported => { + PBAR.info("Skipping wasm-opt because it is not supported on this platform"); + return Ok(()); + } + }; + + PBAR.info("Optimizing wasm binaries with `wasm-opt`..."); + + for file in out_dir.read_dir()? { + let file = file?; + let path = file.path(); + if path.extension().and_then(|s| s.to_str()) != Some("wasm") { + continue; + } + + let tmp = path.with_extension("wasm-opt.wasm"); + let mut cmd = Command::new(&wasm_opt); + cmd.arg(&path).arg("-o").arg(&tmp).args(args); + child::run(cmd, "wasm-opt")?; + std::fs::rename(&tmp, &path)?; + } + + Ok(()) +} + +/// Possible results of `find_wasm_opt` +pub enum WasmOpt { + /// Couldn't install wasm-opt because downloads are forbidden + CannotInstall, + /// The current platform doesn't support precompiled binaries + PlatformNotSupported, + /// We found `wasm-opt` at the specified path + Found(PathBuf), +} + +/// Attempts to find `wasm-opt` in `PATH` locally, or failing that downloads a +/// precompiled binary. +/// +/// Returns `Some` if a binary was found or it was successfully downloaded. +/// Returns `None` if a binary wasn't found in `PATH` and this platform doesn't +/// have precompiled binaries. Returns an error if we failed to download the +/// binary. +pub fn find_wasm_opt(cache: &Cache, install_permitted: bool) -> Result { + // First attempt to look up in PATH. If found assume it works. + if let Ok(path) = which::which("wasm-opt") { + debug!("found wasm-opt at {:?}", path); + return Ok(WasmOpt::Found(path)); + } + + // ... and if that fails download a precompiled version. + let target = if target::LINUX && target::x86_64 { + "x86_64-linux" + } else if target::MACOS && target::x86_64 { + "x86_64-apple-darwin" + } else if target::WINDOWS && target::x86_64 { + "x86_64-windows" + } else { + return Ok(WasmOpt::PlatformNotSupported); + }; + let url = format!( + "https://github.com/WebAssembly/binaryen/releases/download/{vers}/binaryen-{vers}-{target}.tar.gz", + vers = "version_78", + target = target, + ); + + let download = |permit_install| cache.download(permit_install, "wasm-opt", &["wasm-opt"], &url); + + let dl = match download(false)? { + Some(dl) => dl, + None if !install_permitted => return Ok(WasmOpt::CannotInstall), + None => { + let msg = format!("{}Installing wasm-opt...", emoji::DOWN_ARROW); + PBAR.info(&msg); + + match download(install_permitted)? { + Some(dl) => dl, + None => return Ok(WasmOpt::CannotInstall), + } + } + }; + + Ok(WasmOpt::Found(dl.binary("wasm-opt")?)) +} diff --git a/tests/all/main.rs b/tests/all/main.rs index a01018b13..680e2018a 100644 --- a/tests/all/main.rs +++ b/tests/all/main.rs @@ -16,6 +16,7 @@ mod build; mod license; mod lockfile; mod manifest; +mod opt; mod readme; mod test; mod utils; diff --git a/tests/all/opt.rs b/tests/all/opt.rs new file mode 100644 index 000000000..adc9159a4 --- /dev/null +++ b/tests/all/opt.rs @@ -0,0 +1,195 @@ +use assert_cmd::prelude::*; +use predicates::prelude::*; +use utils; + +#[test] +fn off_in_dev() { + let fixture = utils::fixture::Fixture::new(); + fixture + .readme() + .cargo_toml("foo") + .file("src/lib.rs", ""); + fixture.install_local_wasm_bindgen(); + fixture.install_wasm_opt(); + + fixture + .wasm_pack() + .arg("build") + .arg("--dev") + .assert() + .stderr(predicates::str::contains("wasm-opt").not()) + .success(); +} + +#[test] +fn on_in_release() { + let fixture = utils::fixture::Fixture::new(); + fixture + .readme() + .cargo_toml("foo") + .file("src/lib.rs", ""); + fixture.install_local_wasm_bindgen(); + fixture.install_wasm_opt(); + + fixture + .wasm_pack() + .arg("build") + .assert() + .stderr(predicates::str::contains("wasm-opt")) + .success(); +} + +#[test] +fn disable_in_release() { + let fixture = utils::fixture::Fixture::new(); + fixture + .readme() + .file( + "Cargo.toml", + r#" + [package] + authors = [] + description = "" + license = "MIT" + name = "foo" + repository = "" + version = "0.1.0" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + wasm-bindgen = "0.2" + + [package.metadata.wasm-pack.profile.release] + wasm-opt = false + "#, + ) + .file("src/lib.rs", ""); + fixture.install_local_wasm_bindgen(); + fixture.install_wasm_opt(); + + fixture + .wasm_pack() + .arg("build") + .assert() + .stderr(predicates::str::contains("wasm-opt").not()) + .success(); +} + +#[test] +fn enable_in_dev() { + let fixture = utils::fixture::Fixture::new(); + fixture + .readme() + .file( + "Cargo.toml", + r#" + [package] + authors = [] + description = "" + license = "MIT" + name = "foo" + repository = "" + version = "0.1.0" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + wasm-bindgen = "0.2" + + [package.metadata.wasm-pack.profile.dev] + wasm-opt = true + "#, + ) + .file("src/lib.rs", ""); + fixture.install_local_wasm_bindgen(); + fixture.install_wasm_opt(); + + fixture + .wasm_pack() + .arg("build") + .arg("--dev") + .assert() + .stderr(predicates::str::contains( + "Optimizing wasm binaries with `wasm-opt`", + )) + .success(); +} + +#[test] +fn custom_args() { + let fixture = utils::fixture::Fixture::new(); + fixture + .readme() + .file( + "Cargo.toml", + r#" + [package] + authors = [] + description = "" + license = "MIT" + name = "foo" + repository = "" + version = "0.1.0" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + wasm-bindgen = "0.2" + + [package.metadata.wasm-pack.profile.release] + wasm-opt = ['--not-accepted-argument'] + "#, + ) + .file("src/lib.rs", ""); + fixture.install_local_wasm_bindgen(); + fixture.install_wasm_opt(); + + fixture + .wasm_pack() + .arg("build") + .assert() + .stderr(predicates::str::contains("--not-accepted-argument")) + .failure(); +} + +#[test] +fn misconfigured() { + let fixture = utils::fixture::Fixture::new(); + fixture + .readme() + .file( + "Cargo.toml", + r#" + [package] + authors = [] + description = "" + license = "MIT" + name = "foo" + repository = "" + version = "0.1.0" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + wasm-bindgen = "0.2" + + [package.metadata.wasm-pack.profile.release] + wasm-opt = 32 + "#, + ) + .file("src/lib.rs", ""); + fixture.install_local_wasm_bindgen(); + fixture.install_wasm_opt(); + + fixture + .wasm_pack() + .arg("build") + .assert() + .stderr(predicates::str::contains("failed to parse manifest")) + .failure(); +} diff --git a/tests/all/utils/fixture.rs b/tests/all/utils/fixture.rs index fced91c9e..40e828a89 100644 --- a/tests/all/utils/fixture.rs +++ b/tests/all/utils/fixture.rs @@ -236,6 +236,15 @@ impl Fixture { download().unwrap().binary("wasm-bindgen").unwrap() } + pub fn install_wasm_opt(&self) { + static INSTALL_WASM_OPT: Once = ONCE_INIT; + let cache = self.cache(); + + INSTALL_WASM_OPT.call_once(|| { + wasm_pack::opt::find_wasm_opt(&cache, true).unwrap(); + }); + } + /// Download `geckodriver` and return its path. /// /// Takes care to ensure that only one `geckodriver` is downloaded for the whole