Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for automatically executing wasm-opt #625

Merged
merged 2 commits into from
Jul 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions docs/src/cargo-toml-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing "size" and "speed" meta options that we planned in the original design:

# This is the dev profile, but could also be the profiling or release profiles.
[package.metadata.wasm-pack.profile.dev]
# The `wasm-opt` key may be absent, in which case we choose a default
#
# or we can explicitly configure that we *don't* want to run it
wasm-opt = false
# or use our default alias to optimize for size
wasm-opt = "size"
# or use our default alias to optimize for speed
wasm-opt = "speed"
# or give your own custom set of CLI flags
wasm-opt = ["--dce", "--duplicate-function-elimination", "--instrument-memory"]

Is the intention to add support for these in follow up PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True! I forgot to mention that here, but I actually first implemented it as a string being a space-separated list of arguments (like wasm-opt = "-Os -some-other-pass"), and then I read the original issue and liked your idea better.

I figured though it may be best to do that as a separate PR as it should be pretty easy to add on top of this, but rather we'd just want to get the bare bones in here

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
Expand All @@ -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
Expand Down
10 changes: 6 additions & 4 deletions src/child.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,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,
)
}
}
Expand All @@ -54,9 +55,10 @@ pub fn run_capture_stdout(mut command: Command, command_name: &Tool) -> Result<S
Ok(String::from_utf8_lossy(&output.stdout).into_owned())
} else {
bail!(
"failed to execute `{}`: exited with {}",
"failed to execute `{}`: exited with {}\n full command: {:?}",
command_name,
output.status
output.status,
command,
)
}
}
21 changes: 21 additions & 0 deletions src/command/build.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Implementation of the `wasm-pack build` command.

use crate::wasm_opt;
use binary_install::{Cache, Download};
use bindgen;
use build;
Expand Down Expand Up @@ -254,6 +255,7 @@ impl Build {
step_copy_license,
step_install_wasm_bindgen,
step_run_wasm_bindgen,
step_run_wasm_opt,
step_create_json,
]);
steps
Expand Down Expand Up @@ -361,4 +363,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);
wasm_opt::run(
&self.cache,
&self.out_dir,
&args,
self.mode.install_permitted(),
)?;
Ok(())
}
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub mod progressbar;
pub mod readme;
pub mod target;
pub mod test;
pub mod wasm_opt;

use progressbar::ProgressOutput;

Expand Down
31 changes: 31 additions & 0 deletions src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ impl Default for CargoWasmPackProfiles {
pub struct CargoWasmPackProfile {
#[serde(default, rename = "wasm-bindgen")]
wasm_bindgen: CargoWasmPackProfileWasmBindgen,
#[serde(default, rename = "wasm-opt")]
wasm_opt: Option<CargoWasmPackProfileWasmOpt>,
}

#[derive(Default, Deserialize)]
Expand Down Expand Up @@ -242,6 +244,19 @@ impl Crate {
}
}

#[derive(Clone, Deserialize)]
#[serde(untagged)]
enum CargoWasmPackProfileWasmOpt {
Enabled(bool),
ExplicitArgs(Vec<String>),
}

impl Default for CargoWasmPackProfileWasmOpt {
fn default() -> Self {
CargoWasmPackProfileWasmOpt::Enabled(false)
}
}

impl CargoWasmPackProfile {
fn default_dev() -> Self {
CargoWasmPackProfile {
Expand All @@ -250,6 +265,7 @@ impl CargoWasmPackProfile {
demangle_name_section: Some(true),
dwarf_debug_info: Some(false),
},
wasm_opt: None,
}
}

Expand All @@ -260,6 +276,7 @@ impl CargoWasmPackProfile {
demangle_name_section: Some(true),
dwarf_debug_info: Some(false),
},
wasm_opt: Some(CargoWasmPackProfileWasmOpt::Enabled(true)),
}
}

Expand All @@ -270,6 +287,7 @@ impl CargoWasmPackProfile {
demangle_name_section: Some(true),
dwarf_debug_info: Some(false),
},
wasm_opt: Some(CargoWasmPackProfileWasmOpt::Enabled(true)),
}
}

Expand Down Expand Up @@ -309,6 +327,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.
Expand All @@ -325,6 +347,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<Vec<String>> {
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 {
Expand Down
108 changes: 108 additions & 0 deletions src/wasm_opt.rs
Original file line number Diff line number Diff line change
@@ -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<WasmOpt, failure::Error> {
// 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")?))
}
1 change: 1 addition & 0 deletions tests/all/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ mod manifest;
mod readme;
mod test;
mod utils;
mod wasm_opt;
mod webdriver;
13 changes: 13 additions & 0 deletions tests/all/utils/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ impl Fixture {
/// Takes care not to re-install for every fixture, but only the one time
/// for the whole test suite.
pub fn install_local_wasm_bindgen(&self) -> PathBuf {
// If wasm-bindgen is being used then it's very likely wasm-opt is going
// to be used as well.
self.install_wasm_opt();

static INSTALL_WASM_BINDGEN: Once = ONCE_INIT;
let cache = self.cache();
let version = "0.2.37";
Expand All @@ -237,6 +241,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::wasm_opt::find_wasm_opt(&cache, true).unwrap();
});
}

/// Install a local cargo-generate for this fixture.
///
/// Takes care not to re-install for every fixture, but only the one time
Expand Down
Loading