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

Reduce wasm binary size with cargo-xbuild & removing rlib crate-type #33

Merged
merged 99 commits into from
Feb 26, 2020

Conversation

ascjones
Copy link
Collaborator

@ascjones ascjones commented Feb 6, 2020

Closes #32.

This PR significantly reduces the final wasm binary size by custom building rust libcore with the panic_immediate_abort feature which prevents panic strings and formatting code being included in the final executable.

This is achieved using cargo-xbuild.

rlib crate-type

rlib is required to generate the metadata and also for ink-as-dependency. However it also causes bloat in the final Wasm blob so it will be automatically removed if enabled when running cargo contract build.

Likewise if the Cargo.toml doesn't have the rlib in crate-types and the user runs generate-metadata, then that will also be added temporarily in order for build to succeed.

A copy of the users Cargo.toml is made, amended as necessary and written to a temporary directory. Relative paths are rewritten to absolute paths.

todo

  • For generate-metadata modify Cargo.toml to add "rlib" crate-type
  • User option to restore original Cargo.toml if process terminated (superseded)
  • Use tmp dir for amended manifests
  • Convert to xbuild
  • Test all happy and unhappy paths
  • Bug with building after build then cargo clean (file not found)
  • Figure out discrepancy of reported opt file size vs actual
  • Test all ink! examples work
  • Fix Delegator build
  • Get generate-metadata to work
  • Remove .cargo/config linkargs
    Add [profile.release] section, use provided values by default? See Add default [profile.release] values to Cargo.toml if not present #40
  • Add --verbose and --quiet args, quiet by default

@ascjones ascjones marked this pull request as ready for review February 7, 2020 17:43
Copy link
Contributor

@Robbepop Robbepop left a comment

Choose a reason for hiding this comment

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

Very nice! All in all the code looks clean.
Worked for Flipper and ERC20 but not so well for Delegator.
(File not found error.)

robrob@t460p~/p/i/e/delegator> ./build-all.sh 
~/projects/ink/examples/delegator/accumulator ~/projects/ink/examples/delegator
 [1/4] Collecting crate metadata
 [2/4] Building cargo project
 [3/4] Post processing wasm file
ERROR: Can't read from the file: Os { code: 2, kind: NotFound, message: "No such file or directory" }
~/projects/ink/examples/delegator
~/projects/ink/examples/delegator/adder ~/projects/ink/examples/delegator
 [1/4] Collecting crate metadata
 [2/4] Building cargo project
 [3/4] Post processing wasm file
ERROR: Can't read from the file: Os { code: 2, kind: NotFound, message: "No such file or directory" }
~/projects/ink/examples/delegator
~/projects/ink/examples/delegator/subber ~/projects/ink/examples/delegator
 [1/4] Collecting crate metadata
 [2/4] Building cargo project
 [3/4] Post processing wasm file
ERROR: Can't read from the file: Os { code: 2, kind: NotFound, message: "No such file or directory" }
~/projects/ink/examples/delegator
 [1/4] Collecting crate metadata
 [2/4] Building cargo project
 [3/4] Post processing wasm file
 [4/4] Optimizing wasm file
 Original wasm size: 34K, Optimized: 14K
	
Your contract is ready. You can find it here:
/home/robrob/projects/ink/examples/delegator/target/delegator.wasm

@ascjones ascjones changed the title Build with xargo to reduce wsam binary size Build with xargo to reduce wasm binary size Feb 11, 2020
@ascjones
Copy link
Collaborator Author

Using xargo doesn't work with ink/examples/delegator, there is a known issue which causes the top level delegator contract to fail with "conflicting versions" of the custom build libcore.

Since xargo is in maintenance mode I am switching over to cargo-xbuild, which just today added the feature to enable panic_immediate_abort which was previously missing.

I've also opened a PR to make it work with workspace configs (as our contract configs are): rust-osdev/cargo-xbuild#57

Copy link
Contributor

@Robbepop Robbepop left a comment

Choose a reason for hiding this comment

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

LGTM!
You might want to write some release notes (for example into RELEASES.md) for the 0.4 release since it is kinda ... big. :)

src/cmd/build.rs Show resolved Hide resolved
src/cmd/build.rs Show resolved Hide resolved
src/cmd/build.rs Show resolved Hide resolved
&tmp_manifest_path.cargo_arg(),
&target_dir_arg,
"--release",
// "--no-default-features", // Breaks builds for MacOS (linker errors), we should investigate this issue asap!
Copy link
Contributor

Choose a reason for hiding this comment

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

I think --no-default-features is wrong anyway because this tool performs IO, so for example cannot be no_std.

pub fn directory(&self) -> Option<&Path> {
let just_a_file_name =
self.path.iter().collect::<Vec<_>>() == vec![Path::new(MANIFEST_FILE)];
if !just_a_file_name {
Copy link
Contributor

Choose a reason for hiding this comment

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

lol

src/workspace.rs Show resolved Hide resolved
@ascjones ascjones merged commit ec118ff into master Feb 26, 2020
@ascjones ascjones deleted the aj-xargo branch February 26, 2020 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Xargo for building Wasm contracts
2 participants