From 9b5d47f5e105501edaf0a25b81bcbf1575015dbc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 25 Jul 2018 15:39:17 -0700 Subject: [PATCH 1/4] Avoid serializing/reparsing modules during gc Currently the `wasm-gc-api` crate doesn't expose `parity_wasm::Module` as a public dependency which means that whenever we want to run a GC (which is twice per `wasm-bindgen` invocation) we have to serialize and reparse the module a lot! The `wasm-bindgen` has to serialize, `wasm-gc` then parses, `wasm-gc` then serializes, and `wasm-bindgen` then parses. This commit sidesteps all of these operations by ensuring that we always use the same `parity_wasm::Module` instance, even when multiple versions of the `parity_wasm` crate are in use. We'll get a speed boost when they happen to align (which they always should for `wasm-bindgen`), but it'll work even if they aren't aligned (by going through serialization). Concretely on my machine this takes a `wasm-bindgen` invocation from 0.5s to 0.2s, a nice win! --- crates/cli-support/Cargo.toml | 2 +- crates/cli-support/src/js/mod.rs | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/cli-support/Cargo.toml b/crates/cli-support/Cargo.toml index c46902f7..b1991869 100644 --- a/crates/cli-support/Cargo.toml +++ b/crates/cli-support/Cargo.toml @@ -19,5 +19,5 @@ serde_derive = "1.0" serde_json = "1.0" tempfile = "3.0" wasm-bindgen-shared = { path = "../shared", version = '=0.2.14' } -wasm-gc-api = "0.1.8" +wasm-gc-api = "0.1.9" wasmi = "0.3" diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index a4125f19..33d4391b 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -1580,12 +1580,14 @@ impl<'a> Context<'a> { fn gc(&mut self) -> Result<(), Error> { let module = mem::replace(self.module, Module::default()); - let wasm_bytes = parity_wasm::serialize(module)?; - let bytes = wasm_gc::Config::new() + let result = wasm_gc::Config::new() .demangle(self.config.demangle) .keep_debug(self.config.keep_debug || self.config.debug) - .gc(&wasm_bytes)?; - *self.module = deserialize_buffer(&bytes)?; + .run(module, |m| parity_wasm::serialize(m).unwrap())?; + *self.module = match result.into_module() { + Ok(m) => m, + Err(result) => deserialize_buffer(&result.into_bytes()?)?, + }; Ok(()) } From 764302cfcc9dff4d6526ed8880256f6625c58628 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 25 Jul 2018 15:42:44 -0700 Subject: [PATCH 2/4] Reuse the same `parity_wasm::Module` instance for `wasmi` Since `wasmi` already has a public dependency on `parity_wasm` let's just use it! A `clone` is much faster than a serialize + parse, reducing a `wasm-bindgen` invocation on my machine from 0.2s to 0.18s. --- crates/cli-support/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/cli-support/src/lib.rs b/crates/cli-support/src/lib.rs index a0f184ab..b4acef55 100644 --- a/crates/cli-support/src/lib.rs +++ b/crates/cli-support/src/lib.rs @@ -134,7 +134,9 @@ impl Bindgen { // This means that whenever we encounter an import or export we'll // execute a shim function which informs us about its type so we can // then generate the appropriate bindings. - let instance = wasmi::Module::from_buffer(&contents) + // + // TODO: avoid a `clone` here of the module if we can + let instance = wasmi::Module::from_parity_wasm_module(module.clone()) .with_context(|_| "failed to create wasmi module")?; let instance = wasmi::ModuleInstance::new(&instance, &MyResolver) .with_context(|_| "failed to instantiate wasm module")?; From f3942229fe0cac3d52334c044625c823558c7151 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 25 Jul 2018 15:49:50 -0700 Subject: [PATCH 3/4] More reuse of a `parity_wasm::Module` in the test runner This commit updates the test runner to only deserialize a `Module` once and then directly pass it to the `wasm-bindgen` config, avoiding pulling in a public dependency with the same strategy as the `wasm-gc-api` crate for now. This reduces the runtime of this step for `wasm-bindgen-test-runner` from ~0.23s to ~0.19s on my machine. --- crates/cli-support/src/lib.rs | 76 +++++++++++++++---- .../cli/src/bin/wasm-bindgen-test-runner.rs | 28 +++---- 2 files changed, 77 insertions(+), 27 deletions(-) diff --git a/crates/cli-support/src/lib.rs b/crates/cli-support/src/lib.rs index b4acef55..95fbd865 100644 --- a/crates/cli-support/src/lib.rs +++ b/crates/cli-support/src/lib.rs @@ -10,10 +10,12 @@ extern crate wasmi; #[macro_use] extern crate failure; +use std::any::Any; use std::collections::BTreeSet; use std::fmt; use std::fs::File; use std::io::{Read, Write}; +use std::mem; use std::path::{Path, PathBuf}; use failure::{Error, ResultExt}; @@ -24,7 +26,7 @@ mod js; pub mod wasm2es6js; pub struct Bindgen { - path: Option, + input: Input, nodejs: bool, nodejs_experimental_modules: bool, browser: bool, @@ -36,10 +38,17 @@ pub struct Bindgen { keep_debug: bool, } +enum Input { + Path(PathBuf), + Bytes(Vec, String), + Module(Module, String), + None, +} + impl Bindgen { pub fn new() -> Bindgen { Bindgen { - path: None, + input: Input::None, nodejs: false, nodejs_experimental_modules: false, browser: false, @@ -53,7 +62,37 @@ impl Bindgen { } pub fn input_path>(&mut self, path: P) -> &mut Bindgen { - self.path = Some(path.as_ref().to_path_buf()); + self.input = Input::Path(path.as_ref().to_path_buf()); + self + } + + /// Explicitly specify the already parsed input module. + /// + /// Note that this API is a little wonky to avoid tying itself with a public + /// dependency on the `parity-wasm` crate, what we currently use to parse + /// wasm mdoules. + /// + /// If the `module` argument is a `parity_wasm::Module` then it will be used + /// directly. Otherwise it will be passed to `into_bytes` to serialize the + /// module to a vector of bytes, and this will deserialize the module later. + /// + /// Note that even if the argument passed in is a `parity_wasm::Module` it + /// doesn't mean that this won't invoke `into_bytes`, if the `parity_wasm` + /// crate versions are different we'll have to go through serialization. + pub fn input_module( + &mut self, + name: &str, + mut module: T, + into_bytes: impl FnOnce(T) -> Vec, + ) -> &mut Bindgen { + let name = name.to_string(); + if let Some(module) = (&mut module as &mut Any).downcast_mut::() { + let blank = Module::new(Vec::new()); + self.input = Input::Module(mem::replace(module, blank), name); + return self + } + + self.input = Input::Bytes(into_bytes(module), name); self } @@ -107,17 +146,28 @@ impl Bindgen { } fn _generate(&mut self, out_dir: &Path) -> Result<(), Error> { - let input = match self.path { - Some(ref path) => path, - None => bail!("must have a path input for now"), + let (mut module, stem) = match self.input { + Input::None => bail!("must have an input by now"), + Input::Module(ref mut m, ref name) => { + let blank_module = Module::new(Vec::new()); + (mem::replace(m, blank_module), &name[..]) + } + Input::Bytes(ref b, ref name) => { + let module = parity_wasm::deserialize_buffer::(&b) + .context("failed to parse input file as wasm")?; + (module, &name[..]) + } + Input::Path(ref path) => { + let mut contents = Vec::new(); + File::open(&path) + .and_then(|mut f| f.read_to_end(&mut contents)) + .with_context(|_| format!("failed to read `{}`", path.display()))?; + let module = parity_wasm::deserialize_buffer::(&contents) + .context("failed to parse input file as wasm")?; + let stem = path.file_stem().unwrap().to_str().unwrap(); + (module, stem) + } }; - let stem = input.file_stem().unwrap().to_str().unwrap(); - let mut contents = Vec::new(); - File::open(&input) - .and_then(|mut f| f.read_to_end(&mut contents)) - .with_context(|_| format!("failed to read `{}`", input.display()))?; - let mut module = parity_wasm::deserialize_buffer::(&contents) - .with_context(|_| "failed to parse input file as wasm")?; let programs = extract_programs(&mut module) .with_context(|_| "failed to extract wasm-bindgen custom sections")?; diff --git a/crates/cli/src/bin/wasm-bindgen-test-runner.rs b/crates/cli/src/bin/wasm-bindgen-test-runner.rs index 32f9a7c7..36be64c4 100644 --- a/crates/cli/src/bin/wasm-bindgen-test-runner.rs +++ b/crates/cli/src/bin/wasm-bindgen-test-runner.rs @@ -50,16 +50,6 @@ fn rmain() -> Result<(), Error> { fs::create_dir(&tmpdir) .context("creating temporary directory")?; - // For now unconditionally generate wasm-bindgen code tailored for node.js, - // but eventually we'll want more options here for browsers! - let mut b = Bindgen::new(); - b.debug(true) - .nodejs(true) - .input_path(&wasm_file_to_test) - .keep_debug(false) - .generate(&tmpdir) - .context("executing `wasm-bindgen` over the wasm file")?; - let module = wasm_file_to_test.file_stem() .and_then(|s| s.to_str()) .ok_or_else(|| format_err!("invalid filename passed in"))?; @@ -118,12 +108,12 @@ fn rmain() -> Result<(), Error> { // execute, and then those objects are passed into wasm for it to execute // when it sees fit. let mut wasm = Vec::new(); - let wasm_file = tmpdir.join(format!("{}_bg.wasm", module)); - File::open(wasm_file).and_then(|mut f| f.read_to_end(&mut wasm)) + File::open(&wasm_file_to_test) + .and_then(|mut f| f.read_to_end(&mut wasm)) .context("failed to read wasm file")?; - let module = Module::deserialize(&mut &wasm[..]) + let wasm = Module::deserialize(&mut &wasm[..]) .context("failed to deserialize wasm module")?; - if let Some(exports) = module.export_section() { + if let Some(exports) = wasm.export_section() { for export in exports.entries() { if !export.field().starts_with("__wbg_test") { continue @@ -135,6 +125,16 @@ fn rmain() -> Result<(), Error> { // And as a final addendum, exit with a nonzero code if any tests fail. js_to_execute.push_str("if (!cx.run(tests)) exit(1);\n"); + // For now unconditionally generate wasm-bindgen code tailored for node.js, + // but eventually we'll want more options here for browsers! + let mut b = Bindgen::new(); + b.debug(true) + .nodejs(true) + .input_module(module, wasm, |m| parity_wasm::serialize(m).unwrap()) + .keep_debug(false) + .generate(&tmpdir) + .context("executing `wasm-bindgen` over the wasm file")?; + let js_path = tmpdir.join("run.js"); File::create(&js_path) .and_then(|mut f| f.write_all(js_to_execute.as_bytes())) From 0992e45e7fb9648a271bbc228c24ffcd71319ed3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 25 Jul 2018 16:06:47 -0700 Subject: [PATCH 4/4] Use `std::fs` read/write conveniences In addition to being more ergonomic these are much more efficient at reading large files as they preallocate internally. This provides a nice speed boost locally, reducing the overhead of `wasm-bindgen-test-runner` from 0.23s to 0.19s, yay! --- crates/cli-support/src/lib.rs | 19 ++++++------------- crates/cli-support/src/wasm2es6js.rs | 11 ++++------- .../cli/src/bin/wasm-bindgen-test-runner.rs | 10 +++------- crates/cli/src/bin/wasm2es6js.rs | 13 ++++--------- 4 files changed, 17 insertions(+), 36 deletions(-) diff --git a/crates/cli-support/src/lib.rs b/crates/cli-support/src/lib.rs index 95fbd865..7108c70f 100644 --- a/crates/cli-support/src/lib.rs +++ b/crates/cli-support/src/lib.rs @@ -13,8 +13,7 @@ extern crate failure; use std::any::Any; use std::collections::BTreeSet; use std::fmt; -use std::fs::File; -use std::io::{Read, Write}; +use std::fs; use std::mem; use std::path::{Path, PathBuf}; @@ -158,9 +157,7 @@ impl Bindgen { (module, &name[..]) } Input::Path(ref path) => { - let mut contents = Vec::new(); - File::open(&path) - .and_then(|mut f| f.read_to_end(&mut contents)) + let contents = fs::read(&path) .with_context(|_| format!("failed to read `{}`", path.display()))?; let module = parity_wasm::deserialize_buffer::(&contents) .context("failed to parse input file as wasm")?; @@ -232,14 +229,12 @@ impl Bindgen { let extension = if self.nodejs_experimental_modules { "mjs" } else { "js" }; let js_path = out_dir.join(stem).with_extension(extension); - File::create(&js_path) - .and_then(|mut f| f.write_all(reset_indentation(&js).as_bytes())) + fs::write(&js_path, reset_indentation(&js)) .with_context(|_| format!("failed to write `{}`", js_path.display()))?; if self.typescript { let ts_path = out_dir.join(stem).with_extension("d.ts"); - File::create(&ts_path) - .and_then(|mut f| f.write_all(ts.as_bytes())) + fs::write(&ts_path, ts) .with_context(|_| format!("failed to write `{}`", ts_path.display()))?; } @@ -248,14 +243,12 @@ impl Bindgen { if self.nodejs { let js_path = wasm_path.with_extension(extension); let shim = self.generate_node_wasm_import(&module, &wasm_path); - File::create(&js_path) - .and_then(|mut f| f.write_all(shim.as_bytes())) + fs::write(&js_path, shim) .with_context(|_| format!("failed to write `{}`", js_path.display()))?; } let wasm_bytes = parity_wasm::serialize(module)?; - File::create(&wasm_path) - .and_then(|mut f| f.write_all(&wasm_bytes)) + fs::write(&wasm_path, wasm_bytes) .with_context(|_| format!("failed to write `{}`", wasm_path.display()))?; Ok(()) } diff --git a/crates/cli-support/src/wasm2es6js.rs b/crates/cli-support/src/wasm2es6js.rs index 45fb63ec..ad945f89 100644 --- a/crates/cli-support/src/wasm2es6js.rs +++ b/crates/cli-support/src/wasm2es6js.rs @@ -2,8 +2,8 @@ extern crate base64; extern crate tempfile; use std::collections::{HashMap, HashSet}; -use std::fs::File; -use std::io::{self, Read, Write}; +use std::fs; +use std::io; use std::process::Command; use failure::{Error, ResultExt}; @@ -375,8 +375,7 @@ impl Output { let td = tempfile::tempdir()?; let wasm = serialize(self.module)?; let wasm_file = td.as_ref().join("foo.wasm"); - File::create(&wasm_file) - .and_then(|mut f| f.write_all(&wasm)) + fs::write(&wasm_file, wasm) .with_context(|_| format!("failed to write wasm to `{}`", wasm_file.display()))?; let wast_file = td.as_ref().join("foo.wast"); @@ -396,9 +395,7 @@ impl Output { "wasm2asm", )?; - let mut asm_func = String::new(); - File::open(&js_file) - .and_then(|mut f| f.read_to_string(&mut asm_func)) + let asm_func = fs::read_to_string(&js_file) .with_context(|_| format!("failed to read `{}`", js_file.display()))?; let mut make_imports = String::from( diff --git a/crates/cli/src/bin/wasm-bindgen-test-runner.rs b/crates/cli/src/bin/wasm-bindgen-test-runner.rs index 36be64c4..aa908d31 100644 --- a/crates/cli/src/bin/wasm-bindgen-test-runner.rs +++ b/crates/cli/src/bin/wasm-bindgen-test-runner.rs @@ -4,8 +4,7 @@ extern crate wasm_bindgen_cli_support; extern crate parity_wasm; use std::env; -use std::fs::{self, File}; -use std::io::{Write, Read}; +use std::fs; use std::path::PathBuf; use std::process::{self, Command}; @@ -107,9 +106,7 @@ fn rmain() -> Result<(), Error> { // Note that we're collecting *JS objects* that represent the functions to // execute, and then those objects are passed into wasm for it to execute // when it sees fit. - let mut wasm = Vec::new(); - File::open(&wasm_file_to_test) - .and_then(|mut f| f.read_to_end(&mut wasm)) + let wasm = fs::read(&wasm_file_to_test) .context("failed to read wasm file")?; let wasm = Module::deserialize(&mut &wasm[..]) .context("failed to deserialize wasm module")?; @@ -136,8 +133,7 @@ fn rmain() -> Result<(), Error> { .context("executing `wasm-bindgen` over the wasm file")?; let js_path = tmpdir.join("run.js"); - File::create(&js_path) - .and_then(|mut f| f.write_all(js_to_execute.as_bytes())) + fs::write(&js_path, js_to_execute) .context("failed to write JS file")?; // Last but not least, execute `node`! Add an entry to `NODE_PATH` for the diff --git a/crates/cli/src/bin/wasm2es6js.rs b/crates/cli/src/bin/wasm2es6js.rs index 50b16ca8..d171057e 100644 --- a/crates/cli/src/bin/wasm2es6js.rs +++ b/crates/cli/src/bin/wasm2es6js.rs @@ -4,8 +4,7 @@ extern crate docopt; extern crate failure; extern crate wasm_bindgen_cli_support; -use std::fs::File; -use std::io::{Read, Write}; +use std::fs; use std::path::PathBuf; use std::process; @@ -58,9 +57,7 @@ fn main() { } fn rmain(args: &Args) -> Result<(), Error> { - let mut wasm = Vec::new(); - File::open(&args.arg_input) - .and_then(|mut f| f.read_to_end(&mut wasm)) + let wasm = fs::read(&args.arg_input) .with_context(|_| format!("failed to read `{}`", args.arg_input.display()))?; let object = wasm_bindgen_cli_support::wasm2es6js::Config::new() @@ -73,8 +70,7 @@ fn rmain(args: &Args) -> Result<(), Error> { if let Some(ref p) = args.flag_output { let dst = p.with_extension("d.ts"); let ts = object.typescript(); - File::create(&dst) - .and_then(|mut f| f.write_all(ts.as_bytes())) + fs::write(&dst, ts) .with_context(|_| format!("failed to write `{}`", dst.display()))?; } } @@ -83,8 +79,7 @@ fn rmain(args: &Args) -> Result<(), Error> { match args.flag_output { Some(ref p) => { - File::create(p) - .and_then(|mut f| f.write_all(js.as_bytes())) + fs::write(p, js) .with_context(|_| format!("failed to write `{}`", p.display()))?; } None => {