Unverified Commit 3454c125 authored by Michael Müller's avatar Michael Müller Committed by GitHub

Support `optimization-passes` in the release profile (#231)

* Reduce code for `OptimizationPasses`

* Fix typo: zeros ➜ zeroes

* Reduce code for `OptimizationPasses`

* Add log output for optimization flags

* Support `optimization-passes` in the release profile

* Add link to Windows binary releases to Readme

* Improve failed assert message

* Account for `binaryen-rs` behavior

* Link GitHub issue in comment

* Implement comments

* Update `--help`
parent d531cde4
Pipeline #130191 failed with stages
in 3 minutes and 59 seconds
......@@ -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`:_
......
......@@ -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<OptimizationPasses>,
}
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(())
})
}
}
......@@ -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<Self, Self::Error> {
match value.optimization_passes.to_lowercase().as_str() {
fn from_str(input: &str) -> std::result::Result<Self, Self::Err> {
// 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<std::string::String> 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)]
......
......@@ -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<OptimizationPasses> {
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<Option<value::Value>> {
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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment