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

Actualizing of the size of the delegator example #1054

Conversation

xgreenx
Copy link
Collaborator

@xgreenx xgreenx commented Nov 28, 2021

Extracted that change from here.

Using workspace forces the compiler to keep stuff for debugging. It increases the size of adder, subber, accumulator.

accumulator: Original wasm size: 26.8K, Optimized: 6.2K -> Original wasm size: 20.5K, Optimized: 1.7K
subber: Original wasm size: 28.4K, Optimized: 7.4K -> Original wasm size: 21.5K, Optimized: 2.9K
adder: Original wasm size: 28.4K, Optimized: 7.4K -> Original wasm size: 21.5K, Optimized: 2.9K

I think it is better to remove it to track the actual size of these contracts.
CI fails with that change due to a strange error. I think it is related to caching, so created that PR separately. Hope you will find the reason=)

@codecov-commenter
Copy link

codecov-commenter commented Nov 28, 2021

Codecov Report

Merging #1054 (6f0071f) into master (141673b) will decrease coverage by 16.21%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1054       +/-   ##
===========================================
- Coverage   78.78%   62.57%   -16.22%     
===========================================
  Files         248      248               
  Lines        9363     9361        -2     
===========================================
- Hits         7377     5858     -1519     
- Misses       1986     3503     +1517     
Impacted Files Coverage Δ
crates/lang/codegen/src/traits.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/ir/src/ir/contract.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/env.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/mod.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/enforced_error.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/blake2b.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/storage.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/arg_list.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/contract.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/selector.rs 0.00% <0.00%> (-100.00%) ⬇️
... and 56 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 141673b...6f0071f. Read the comment docs.

@Robbepop
Copy link
Collaborator

Thanks for debugging this. I was already wondering why those delegator sub contracts were always a lot bigger than for example the very similar Incrementer example contracts.

@ascjones @cmichi Is there a way me might be able to mitigate this through a pass via cargo-contract?

@cmichi
Copy link
Collaborator

cmichi commented Nov 29, 2021

Hey those are some really nice results, @xgreenx !

I've debugged the CI failure and I think it was just some temporary thing, the CI in #1055 builds all contracts fine. The results are not yet posted there due to some issue with the posting bot, I'll fix that next.

@ascjones @cmichi Is there a way me might be able to mitigate this through a pass via cargo-contract?

Yes, I think that's the right approach and I've created use-ink/cargo-contract#377 for it. @xgreenx Are you up for the issue on the cargo-contract side?

@xgreenx
Copy link
Collaborator Author

xgreenx commented Nov 29, 2021

Are you up for the issue on the cargo-contract side?

I don't know how to ignore the [workspace] from parent Cargo.toml=) The problem is that Cargo.toml in the delegate folder affects the build process in the delegatore/adder folder. I will try to investigate that question but I'm not sure about the result=)

@xgreenx
Copy link
Collaborator Author

xgreenx commented Nov 29, 2021

use-ink/cargo-contract#378 fixes the issue=) So, I think we can close this PR and wait for the fix from cargo-contract.

@paritytech-ci
Copy link

paritytech-ci commented Nov 29, 2021

🦑 📈 ink! Example Contracts ‒ Size Change Report 📉 🦑

These are the results of building the examples/* contracts from this branch with cargo-contract 0.16.0:

Δ Original Size Δ Optimized Size Total Optimized Size
accumulator 20.52 K
adder 21.49 K
contract-terminate 20.03 K
contract-transfer 28.02 K
delegator 32.57 K
dns 51.79 K
erc1155 88.03 K
erc20 -0.06 K -0.06 K 34.31 K
erc721 84.63 K
flipper 20.95 K
incrementer 20.72 K
multisig 105.32 K
proxy 23.86 K
rand-extension 26.52 K
subber 21.51 K
trait-erc20 64.41 K
trait-flipper 20.67 K
trait-incrementer 20.69 K

Link to the run | Last update: Tue Nov 30 08:24:58 CET 2021

@xgreenx
Copy link
Collaborator Author

xgreenx commented Nov 29, 2021

@cmichi I thought the CI uses the latest cargo-contract=)

@cmichi
Copy link
Collaborator

cmichi commented Nov 30, 2021

@cmichi I thought the CI uses the latest cargo-contract=)

Yep, it does, but the CI docker container is only rebuild nightly ‒ so in the case of your cargo-contract merge yesterday it took some time for that carg-contract master version to appear in the ink! CI. If you now look at the GitLab logs you'll see

$ cargo-contract --version
cargo-contract 0.16.0-3b5d562-x86_64-linux-gnu

(which is the latest cargo-contract HEAD)

The sizes posted by the bot above are when this PR here is compared against ink! master ‒ which now doesn't yield any difference anymore, since master already uses your merged cargo-contract PR.

The bot above still shows one number, but that's only because there is some small noise in one of the late decimal points, you can ignore that.

I'll close this issue now, since it has been solved in cargo-contract.

@cmichi cmichi closed this Nov 30, 2021
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.

5 participants