diff --git a/README.md b/README.md index fb4c0d94aa2cbaa62b140146486208cccd20cad3..41a86bab872c4950237de47a6b03cac304123f83 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ We optimize the resulting contract Wasm using `binaryen`. You have two options f Install [`binaryen`](https://github.com/WebAssembly/binaryen#tools) with a version >= 99. Many package managers have it available nowadays ‒ e.g. it's a package for [Debian/Ubuntu](https://tracker.debian.org/pkg/binaryen), [Homebrew](https://formulae.brew.sh/formula/binaryen) and [Arch Linux](https://archlinux.org/packages/community/x86_64/binaryen/). + For Windows, [binary releases are available](https://github.com/WebAssembly/binaryen/releases). After you've installed the package execute `cargo install --force cargo-contract`. - _Build `binaryen` as a dependency when installing `cargo-contract`:_ diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 60efcd5bee0923220be86ed4752c58b1d1a56321..abef85ef01a69ceca3d93e61cb849b4315433382 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -28,9 +28,9 @@ use std::{process::Command, str}; use crate::{ crate_metadata::CrateMetadata, maybe_println, util, validate_wasm, - workspace::{ManifestPath, Profile, Workspace}, - BuildArtifacts, BuildResult, OptimizationFlags, OptimizationPasses, OptimizationResult, - UnstableFlags, UnstableOptions, Verbosity, VerbosityFlags, + workspace::{Manifest, ManifestPath, Profile, Workspace}, + BuildArtifacts, BuildResult, OptimizationPasses, OptimizationResult, UnstableFlags, + UnstableOptions, Verbosity, VerbosityFlags, }; use anyhow::{Context, Result}; use colored::Colorize; @@ -66,8 +66,30 @@ pub struct BuildCommand { verbosity: VerbosityFlags, #[structopt(flatten)] unstable_options: UnstableOptions, - #[structopt(flatten)] - optimization_passes: OptimizationFlags, + /// Number of optimization passes, passed as an argument to wasm-opt. + /// + /// - `0`: execute no optimization passes + /// + /// - `1`: execute 1 optimization pass (quick & useful opts, useful for iteration builds) + /// + /// - `2`, execute 2 optimization passes (most opts, generally gets most perf) + /// + /// - `3`, execute 3 optimization passes (spends potentially a lot of time optimizing) + /// + /// - `4`, execute 4 optimization passes (also flatten the IR, which can take a lot more time and memory + /// but is useful on more nested / complex / less-optimized input) + /// + /// - `s`, execute default optimization passes, focusing on code size + /// + /// - `z`, execute default optimization passes, super-focusing on code size + /// + /// - The default value is `3` + /// + /// - It is possible to define the number of optimization passes in the + /// `[package.metadata.contract]` of your `Cargo.toml` as e.g. `optimization-passes = "3"`. + /// The CLI argument always takes precedence over the profile value. + #[structopt(long = "optimization-passes")] + optimization_passes: Option, } impl BuildCommand { @@ -76,8 +98,22 @@ impl BuildCommand { let unstable_flags: UnstableFlags = TryFrom::<&UnstableOptions>::try_from(&self.unstable_options)?; let verbosity = TryFrom::<&VerbosityFlags>::try_from(&self.verbosity)?; - let optimization_passes = - TryFrom::<&OptimizationFlags>::try_from(&self.optimization_passes)?; + + // The CLI flag `optimization-passes` overwrites optimization passes which are + // potentially defined in the `Cargo.toml` profile. + let optimization_passes = match self.optimization_passes { + Some(opt_passes) => opt_passes, + None => { + let mut manifest = Manifest::new(manifest_path.clone())?; + match manifest.get_profile_optimization_passes() { + // if no setting is found, neither on the cli nor in the profile, + // then we use the default + None => OptimizationPasses::default(), + Some(opt_passes) => opt_passes, + } + } + }; + execute( &manifest_path, verbosity, @@ -106,13 +142,12 @@ impl CheckCommand { let unstable_flags: UnstableFlags = TryFrom::<&UnstableOptions>::try_from(&self.unstable_options)?; let verbosity: Verbosity = TryFrom::<&VerbosityFlags>::try_from(&self.verbosity)?; - let optimization_passes = OptimizationPasses::Zero; execute( &manifest_path, verbosity, BuildArtifacts::CheckOnly, unstable_flags, - optimization_passes, + OptimizationPasses::Zero, ) } } @@ -318,9 +353,23 @@ fn do_optimization( // The default debug_info: false, }; + log::info!( + "Optimization level passed to `binaryen` dependency: {}", + codegen_config.optimization_level + ); + log::info!( + "Shrink level passed to `binaryen` dependency: {}", + codegen_config.shrink_level + ); let mut module = binaryen::Module::read(&dest_wasm_file_content) .map_err(|_| anyhow::anyhow!("binaryen failed to read file content"))?; - module.optimize(&codegen_config); + + if optimization_level != OptimizationPasses::Zero { + // binaryen-rs still uses the default optimization passes, even if zero + // is passed. this is the ticket for it: https://github.com/pepyakin/binaryen-rs/issues/56. + // we can remove the if condition here once the issue is fixed. + module.optimize(&codegen_config); + } let mut optimized_wasm_file = File::create(dest_optimized)?; optimized_wasm_file.write_all(&module.write())?; @@ -352,13 +401,17 @@ fn do_optimization( ); } + log::info!( + "Optimization level passed to wasm-opt: {}", + optimization_level + ); let output = Command::new("wasm-opt") .arg(dest_wasm) - .arg(format!("-O{}", optimization_level.to_str())) + .arg(format!("-O{}", optimization_level)) .arg("-o") .arg(dest_optimized) // the memory in our module is imported, `wasm-opt` needs to be told that - // the memory is initialized to zeros, otherwise it won't run the + // the memory is initialized to zeroes, otherwise it won't run the // memory-packing pre-pass. .arg("--zero-filled-memory") .output()?; @@ -454,8 +507,11 @@ pub(crate) fn execute( #[cfg(test)] mod tests_ci_only { use crate::{ - cmd, util::tests::with_tmp_dir, BuildArtifacts, ManifestPath, OptimizationPasses, - UnstableFlags, Verbosity, + cmd::{self, BuildCommand}, + util::tests::with_tmp_dir, + workspace::Manifest, + BuildArtifacts, ManifestPath, OptimizationPasses, UnstableFlags, UnstableOptions, + Verbosity, VerbosityFlags, }; #[test] @@ -526,4 +582,48 @@ mod tests_ci_only { Ok(()) }) } + + #[test] + fn cli_optimization_passes_must_take_precedence_over_profile() { + with_tmp_dir(|path| { + // given + cmd::new::execute("new_project", Some(path)).expect("new project creation failed"); + let cargo_toml_path = path.join("new_project").join("Cargo.toml"); + let manifest_path = + ManifestPath::new(&cargo_toml_path).expect("manifest path creation failed"); + // we write "4" as the optimization passes into the release profile + assert!(Manifest::new(manifest_path.clone())? + .set_profile_optimization_passes(String::from("4").into()) + .is_ok()); + let cmd = BuildCommand { + manifest_path: Some(cargo_toml_path), + build_artifact: BuildArtifacts::All, + verbosity: VerbosityFlags::default(), + unstable_options: UnstableOptions::default(), + + // we choose zero optimization passes as the "cli" parameter + optimization_passes: Some(OptimizationPasses::Zero), + }; + + // when + let res = cmd.exec().expect("build failed"); + let optimization = res + .optimization_result + .expect("no optimization result available"); + + // then + // we have to truncate here to account for a possible small delta + // in the floating point numbers + let optimized_size = optimization.optimized_size.trunc(); + let original_size = optimization.original_size.trunc(); + assert!( + optimized_size == original_size, + "The optimized size {:?} differs from the original size {:?}", + optimized_size, + original_size + ); + + Ok(()) + }) + } } diff --git a/src/main.rs b/src/main.rs index b34c10b064884c167e0693d107f96714a18ffed4..326c3d69f3ad9153810b2c539ff7a4c9e288e339 100644 --- a/src/main.rs +++ b/src/main.rs @@ -26,7 +26,12 @@ use crate::cmd::{metadata::MetadataResult, BuildCommand, CheckCommand}; #[cfg(feature = "extrinsics")] use sp_core::{crypto::Pair, sr25519, H256}; -use std::{convert::TryFrom, path::PathBuf}; +use std::{ + convert::TryFrom, + fmt::{Display, Formatter, Result as DisplayResult}, + path::PathBuf, + str::FromStr, +}; #[cfg(feature = "extrinsics")] use subxt::PairSigner; @@ -93,31 +98,8 @@ impl ExtrinsicOpts { } } -#[derive(Clone, Debug, StructOpt)] -pub struct OptimizationFlags { - /// Number of optimization passes, passed as an argument to wasm-opt. - /// - /// - `0`: execute no optimization passes - /// - /// - `1`: execute 1 optimization pass (quick & useful opts, useful for iteration builds) - /// - /// - `2`, execute 2 optimization passes (most opts, generally gets most perf) - /// - /// - `3`, execute 3 optimization passes (spends potentially a lot of time optimizing) - /// - /// - `4`, execute 4 optimization passes (also flatten the IR, which can take a lot more time and memory - /// but is useful on more nested / complex / less-optimized input) - /// - /// - `s`, execute default optimization passes, focusing on code size - /// - /// - `z`, execute default optimization passes, super-focusing on code size - /// - /// - - #[structopt(long = "optimization-passes", default_value = "3")] - optimization_passes: String, -} - -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] +#[cfg_attr(test, derive(PartialEq))] pub enum OptimizationPasses { Zero, One, @@ -128,17 +110,36 @@ pub enum OptimizationPasses { Z, } +impl Display for OptimizationPasses { + fn fmt(&self, f: &mut Formatter<'_>) -> DisplayResult { + let out = match self { + OptimizationPasses::Zero => "0", + OptimizationPasses::One => "1", + OptimizationPasses::Two => "2", + OptimizationPasses::Three => "3", + OptimizationPasses::Four => "4", + OptimizationPasses::S => "s", + OptimizationPasses::Z => "z", + }; + write!(f, "{}", out) + } +} + impl Default for OptimizationPasses { fn default() -> OptimizationPasses { OptimizationPasses::Three } } -impl TryFrom<&OptimizationFlags> for OptimizationPasses { - type Error = Error; +impl std::str::FromStr for OptimizationPasses { + type Err = Error; - fn try_from(value: &OptimizationFlags) -> Result { - match value.optimization_passes.to_lowercase().as_str() { + fn from_str(input: &str) -> std::result::Result { + // We need to replace " here, since the input string could come + // from either the CLI or the `Cargo.toml` profile section. + // If it is from the profile it could e.g. be "3" or 3. + let normalized_input = input.replace("\"", "").to_lowercase(); + match normalized_input.as_str() { "0" => Ok(OptimizationPasses::Zero), "1" => Ok(OptimizationPasses::One), "2" => Ok(OptimizationPasses::Two), @@ -146,29 +147,18 @@ impl TryFrom<&OptimizationFlags> for OptimizationPasses { "4" => Ok(OptimizationPasses::Four), "s" => Ok(OptimizationPasses::S), "z" => Ok(OptimizationPasses::Z), - _ => anyhow::bail!( - "Unknown optimization passes option {}", - value.optimization_passes - ), + _ => anyhow::bail!("Unknown optimization passes for option {}", input), } } } -impl OptimizationPasses { - /// Returns the string representation of `OptimizationPasses` - #[cfg(not(feature = "binaryen-as-dependency"))] - pub(crate) fn to_str(&self) -> &str { - match self { - OptimizationPasses::Zero => "0", - OptimizationPasses::One => "1", - OptimizationPasses::Two => "2", - OptimizationPasses::Three => "3", - OptimizationPasses::Four => "4", - OptimizationPasses::S => "s", - OptimizationPasses::Z => "z", - } +impl From for OptimizationPasses { + fn from(str: String) -> Self { + OptimizationPasses::from_str(&str).expect("conversion failed") } +} +impl OptimizationPasses { /// Returns the number of optimization passes to do #[cfg(feature = "binaryen-as-dependency")] pub(crate) fn to_passes(&self) -> u32 { @@ -194,7 +184,7 @@ impl OptimizationPasses { } } -#[derive(Clone, Debug, StructOpt)] +#[derive(Default, Clone, Debug, StructOpt)] pub struct VerbosityFlags { /// No output printed to stdout #[structopt(long)] @@ -238,7 +228,7 @@ impl TryFrom<&VerbosityFlags> for Verbosity { } } -#[derive(Clone, Debug, StructOpt)] +#[derive(Default, Clone, Debug, StructOpt)] struct UnstableOptions { /// Use the original manifest (Cargo.toml), do not modify for build optimizations #[structopt(long = "unstable-options", short = "Z", number_of_values = 1)] diff --git a/src/workspace/manifest.rs b/src/workspace/manifest.rs index dc1699bba8fe3394c5e76d1b1250afd8ebc02b16..ce44c030cd9ef7707c87cf564d1342fcaa04b9f2 100644 --- a/src/workspace/manifest.rs +++ b/src/workspace/manifest.rs @@ -17,6 +17,8 @@ use anyhow::{Context, Result}; use super::{metadata, Profile}; +use crate::OptimizationPasses; + use std::convert::TryFrom; use std::{ collections::HashSet, @@ -160,6 +162,48 @@ impl Manifest { Ok(self) } + /// Extract `optimization-passes` from `[package.metadata.contract]` + pub fn get_profile_optimization_passes(&mut self) -> Option { + self.toml + .get("package")? + .as_table()? + .get("metadata")? + .as_table()? + .get("contract")? + .as_table()? + .get("optimization-passes") + .map(|val| val.to_string()) + .map(Into::into) + } + + /// Set `optimization-passes` in `[package.metadata.contract]` + #[cfg(test)] + pub fn set_profile_optimization_passes( + &mut self, + passes: OptimizationPasses, + ) -> Result> { + Ok(self + .toml + .entry("package") + .or_insert(value::Value::Table(Default::default())) + .as_table_mut() + .ok_or(anyhow::anyhow!("package section should be a table"))? + .entry("metadata") + .or_insert(value::Value::Table(Default::default())) + .as_table_mut() + .ok_or(anyhow::anyhow!("metadata section should be a table"))? + .entry("contract") + .or_insert(value::Value::Table(Default::default())) + .as_table_mut() + .ok_or(anyhow::anyhow!( + "metadata.contract section should be a table" + ))? + .insert( + "optimization-passes".to_string(), + value::Value::String(passes.to_string()), + )) + } + /// Set `[profile.release]` lto flag pub fn with_profile_release_lto(&mut self, enabled: bool) -> Result<&mut Self> { let lto = self