From 442573ea1a293540be15e5344aead048278b9dbf Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Tue, 16 Feb 2021 17:50:17 +0100 Subject: [PATCH 1/6] Revert me: Hotfix for funty issue --- Cargo.lock | 3 +++ Cargo.toml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index e5c48a6f..18ea419b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,7 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +version = 3 + [[package]] name = "Inflector" version = "0.11.4" @@ -489,6 +491,7 @@ dependencies = [ "colored", "contract-metadata", "env_logger", + "funty", "futures", "heck", "hex", diff --git a/Cargo.toml b/Cargo.toml index 420a9ba9..7883206c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,9 @@ subxt = { version = "0.14.0", package = "substrate-subxt", optional = true } futures = { version = "0.3.12", optional = true } hex = { version = "0.4.2", optional = true } +# Should be removed once bitvecto-rs/bitvec#105 is resolved +funty = "=1.1.0" + [build-dependencies] anyhow = "1.0.38" zip = { version = "0.5.10", default-features = false } -- GitLab From fb6bcfd11ffaf27830feae3bcc5d3d34bbdebf02 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 18 Feb 2021 12:40:46 +0100 Subject: [PATCH 2/6] Assert that size of resulting wasm is > 0 --- src/cmd/build.rs | 6 ++++++ src/main.rs | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 98060919..e9257863 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -254,6 +254,11 @@ fn post_process_wasm(crate_metadata: &CrateMetadata) -> Result<()> { validate_wasm::validate_import_section(&module)?; + debug_assert!( + !module.clone().to_bytes().unwrap().is_empty(), + "resulting wasm size of post processing must be > 0" + ); + parity_wasm::serialize_to_file(&crate_metadata.dest_wasm, module)?; Ok(()) } @@ -462,6 +467,7 @@ mod tests_ci_only { // the path can never be e.g. `foo_target/ink` -- the assert // would fail for that. assert!(res.target_directory.ends_with("target/ink")); + assert!(res.optimization_result.unwrap().optimized_size > 0.0); Ok(()) }) } diff --git a/src/main.rs b/src/main.rs index 0d4c63e5..445d4886 100644 --- a/src/main.rs +++ b/src/main.rs @@ -238,6 +238,10 @@ impl BuildResult { format!("{:.1}K", optimization.0).bold(), format!("{:.1}K", optimization.1).bold(), ); + debug_assert!( + optimization.1 > 0.0, + "optimized file size must be greater 0" + ); if self.build_artifact == BuildArtifacts::CodeOnly { let out = format!( -- GitLab From de018dffe4bc3cf7211cb34b880654cf70b4dd4d Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 18 Feb 2021 12:43:59 +0100 Subject: [PATCH 3/6] Ensure optimized file is not overwritten with empty file on `not(feature = binaryen-as-dependency)` --- src/cmd/build.rs | 70 +++++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index e9257863..2cbf3dd7 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -14,23 +14,24 @@ // You should have received a copy of the GNU General Public License // along with cargo-contract. If not, see . +use std::{convert::TryFrom, ffi::OsStr, fs::metadata, path::PathBuf}; + +#[cfg(feature = "binaryen-as-dependency")] use std::{ - convert::TryFrom, - fs::{metadata, File}, + fs::File, io::{Read, Write}, - path::{Path, PathBuf}, }; #[cfg(not(feature = "binaryen-as-dependency"))] -use std::{io, process::Command}; +use std::process::Command; use crate::{ crate_metadata::CrateMetadata, maybe_println, util, validate_wasm, workspace::{ManifestPath, Profile, Workspace}, - BuildArtifacts, BuildResult, UnstableFlags, UnstableOptions, VerbosityFlags, + BuildArtifacts, BuildResult, OptimizationResult, UnstableFlags, UnstableOptions, Verbosity, + VerbosityFlags, }; -use crate::{OptimizationResult, Verbosity}; use anyhow::{Context, Result}; use colored::Colorize; use parity_wasm::elements::{External, MemoryType, Module, Section}; @@ -268,23 +269,20 @@ fn post_process_wasm(crate_metadata: &CrateMetadata) -> Result<()> { /// The intention is to reduce the size of bloated wasm binaries as a result of missing /// optimizations (or bugs?) between Rust and Wasm. fn optimize_wasm(crate_metadata: &CrateMetadata) -> Result { - let mut optimized = crate_metadata.dest_wasm.clone(); - optimized.set_file_name(format!("{}-opt.wasm", crate_metadata.package_name)); - - let mut dest_wasm_file = File::open(crate_metadata.dest_wasm.as_os_str())?; - let mut dest_wasm_file_content = Vec::new(); - dest_wasm_file.read_to_end(&mut dest_wasm_file_content)?; - - let optimized_wasm = do_optimization(crate_metadata, &optimized, &dest_wasm_file_content, 3)?; + let mut dest_optimized = crate_metadata.dest_wasm.clone(); + dest_optimized.set_file_name(format!("{}-opt.wasm", crate_metadata.package_name)); - let mut optimized_wasm_file = File::create(optimized.as_os_str())?; - optimized_wasm_file.write_all(&optimized_wasm)?; + let _ = do_optimization( + crate_metadata.dest_wasm.as_os_str(), + &dest_optimized.as_os_str(), + 3, + )?; let original_size = metadata(&crate_metadata.dest_wasm)?.len() as f64 / 1000.0; - let optimized_size = metadata(&optimized)?.len() as f64 / 1000.0; + let optimized_size = metadata(&dest_optimized)?.len() as f64 / 1000.0; // overwrite existing destination wasm file with the optimised version - std::fs::rename(&optimized, &crate_metadata.dest_wasm)?; + std::fs::rename(&dest_optimized, &crate_metadata.dest_wasm)?; Ok(OptimizationResult { original_size, optimized_size, @@ -299,23 +297,30 @@ fn optimize_wasm(crate_metadata: &CrateMetadata) -> Result { /// If successful, the optimized Wasm is returned as a `Vec`. #[cfg(feature = "binaryen-as-dependency")] fn do_optimization( - _: &CrateMetadata, - _: &Path, - wasm: &[u8], + dest_wasm: &OsStr, + dest_optimized: &OsStr, optimization_level: u32, -) -> Result> { +) -> Result<()> { + let mut dest_wasm_file = File::open(dest_wasm)?; + let mut dest_wasm_file_content = Vec::new(); + dest_wasm_file.read_to_end(&mut dest_wasm_file_content)?; + let codegen_config = binaryen::CodegenConfig { // number of optimization passes (spends potentially a lot of time optimizing) optimization_level, // the default shrink_level: 1, // the default - debug_info: false, + debug_info: true, }; - let mut module = binaryen::Module::read(&wasm) + let mut module = binaryen::Module::read(&dest_wasm_file_content) .map_err(|_| anyhow::anyhow!("binaryen failed to read file content"))?; module.optimize(&codegen_config); - Ok(module.write()) + + let mut optimized_wasm_file = File::create(dest_optimized)?; + optimized_wasm_file.write_all(&module.write())?; + + Ok(()) } /// Optimizes the Wasm supplied as `crate_metadata.dest_wasm` using @@ -328,11 +333,10 @@ fn do_optimization( /// and returned as a `Vec`. #[cfg(not(feature = "binaryen-as-dependency"))] fn do_optimization( - crate_metadata: &CrateMetadata, - optimized_dest: &Path, - _: &[u8], + dest_wasm: &OsStr, + dest_optimized: &OsStr, optimization_level: u32, -) -> Result> { +) -> Result<()> { // check `wasm-opt` is installed if which::which("wasm-opt").is_err() { anyhow::bail!( @@ -345,19 +349,17 @@ fn do_optimization( } let output = Command::new("wasm-opt") - .arg(crate_metadata.dest_wasm.as_os_str()) + .arg(dest_wasm) .arg(format!("-O{}", optimization_level)) .arg("-o") - .arg(optimized_dest.as_os_str()) + .arg(dest_optimized) .output()?; if !output.status.success() { // Dump the output streams produced by `wasm-opt` into the stdout/stderr. - io::stdout().write_all(&output.stdout)?; - io::stderr().write_all(&output.stderr)?; anyhow::bail!("wasm-opt optimization failed"); } - Ok(output.stdout) + Ok(()) } /// Executes build of the smart-contract which produces a wasm binary that is ready for deploying. -- GitLab From 1fceeb8fbb61b0116a2dc9bb0fa3ffcb9b5190c5 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 18 Feb 2021 12:49:55 +0100 Subject: [PATCH 4/6] Fix `warning: panic message is not a string literal` --- src/cmd/metadata.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index 0197c2f1..4a20bb31 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -390,7 +390,8 @@ mod tests { assert!( dest_bundle.exists(), - format!("Missing metadata file '{}'", dest_bundle.display()) + "Missing metadata file '{}'", + dest_bundle.display() ); let source = metadata_json.get("source").expect("source not found"); -- GitLab From ab7468f02eb1b5b57c3ec7f9905c97919cbcc5d1 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 18 Feb 2021 13:04:47 +0100 Subject: [PATCH 5/6] Do not record debug info --- src/cmd/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 2cbf3dd7..de8336db 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -311,7 +311,7 @@ fn do_optimization( // the default shrink_level: 1, // the default - debug_info: true, + debug_info: false, }; let mut module = binaryen::Module::read(&dest_wasm_file_content) .map_err(|_| anyhow::anyhow!("binaryen failed to read file content"))?; -- GitLab From a27ea6e1aed0c7d565857504ad25bbca486c3e7c Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 18 Feb 2021 14:07:25 +0100 Subject: [PATCH 6/6] Update comments --- src/cmd/build.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index de8336db..2a03e774 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -294,7 +294,7 @@ fn optimize_wasm(crate_metadata: &CrateMetadata) -> Result { /// The supplied `optimization_level` denotes the number of optimization passes, /// resulting in potentially a lot of time spent optimizing. /// -/// If successful, the optimized Wasm is returned as a `Vec`. +/// If successful, the optimized wasm is written to `dest_optimized`. #[cfg(feature = "binaryen-as-dependency")] fn do_optimization( dest_wasm: &OsStr, @@ -329,8 +329,7 @@ fn do_optimization( /// The supplied `optimization_level` denotes the number of optimization passes, /// resulting in potentially a lot of time spent optimizing. /// -/// If successful, the optimized Wasm file is created under `optimized` -/// and returned as a `Vec`. +/// If successful, the optimized wasm is written to `dest_optimized`. #[cfg(not(feature = "binaryen-as-dependency"))] fn do_optimization( dest_wasm: &OsStr, -- GitLab