use-ink
Reworked message dispatch logic
In this PR I moved common parts of each message to execute_dispatchable(pull of the contract). In the future, we can optimize it to do one push of the contract.
Introduced an additional static buffer to store input. Now the user has access to the input buffer at any time during the execution of the contract(it is loaded only one time). This buffer is needed to rework the decoding process.
Moved the decode logic to the CALLABLE part. It allows the compiler to use value after decoding and do some optimization.
Call unwrap instead of passing error upper during execute dispatchable and decoding. It allows the compiler to optimize the code better.
Optimized return_value function to simply use buffer for encoding the result. We don't need complex methods from ScoppedBuffer because after the call of that method the program will exit.
All these optimizations allow reducing the code of optimized contracts well.
Erc20 takes 7862 bytes instead of 10282(saved 2420).
The more methods in the contract, the stronger the effect of the change.
That contract contains a lot of methods and the chagne saved 6489 bytes:
Original wasm size: 71.8K, Optimized: 38.8K(38844) -> Original wasm size: 81.7K, Optimized: 32.4K(32355)
@xgreenx You can get the ink-waterfall stage to run by removing the failing CI checks altogether from .gitlab-ci.yml. So just remove all failing stages (clippy-std, clippy-wasm, etc.) and push these changes to this PR.
@xgreenx You can get the
ink-waterfallstage to run by removing the failing CI checks altogether from.gitlab-ci.yml. So just remove all failing stages (clippy-std,clippy-wasm, etc.) and push these changes to this PR.
ink-waterfall failed=( tests::delegator::delegator_works. I tried it on local node and it works(manually, didn't try to run a test). Maybe it is not enough gas?
@xgreenx You can get the
ink-waterfallstage to run by removing the failing CI checks altogether from.gitlab-ci.yml. So just remove all failing stages (clippy-std,clippy-wasm, etc.) and push these changes to this PR.
ink-waterfallfailed=(tests::delegator::delegator_works. I tried it on local node and it works(manually, didn't try to run a test). Maybe it is not enough gas?
Oh, after my change the contract requires 53k additional gas=D Because I'm allocating 16MB in stack=D But I think we can decide in this PR how to optimize it(for example use static buffer form instance)
🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑
These are the results when building the examples/* contracts from this branch with cargo-contract 0.17.0-06c5598 and comparing them to ink! master:
| Δ Optimized Size | Δ Used Gas | Total Optimized Size | Total Used Gas | |
|---|---|---|---|---|
accumulator |
+1.62 K | 1.62 K | ||
adder |
+2.82 K | 2.82 K | ||
contract-terminate |
-0.12 K | -1 | 1.19 K | 200_777 |
contract-transfer |
-0.05 K | +16 | 7.94 K | 1_415 |
delegator |
-0.86 K | -1_063 | 9.20 K | 8_447 |
dns |
-1.14 K | -1_816 | 9.33 K | 12_472 |
erc1155 |
-2.88 K | -3_718 | 24.62 K | 44_093 |
erc20 |
-2.06 K | -1_929 | 8.16 K | 7_828 |
erc721 |
-2.44 K | +1_264 | 11.26 K | 35_966 |
flipper |
+0.04 K | +21 | 1.81 K | 368 |
incrementer |
+0.05 K | +23 | 1.69 K | 363 |
multisig |
-1.20 K | -6_313 | 27.36 K | 48_583 |
proxy |
-0.23 K | -80 | 3.58 K | 2_549 |
rand-extension |
-1.07 K | -92 | 4.04 K | 6_562 |
subber |
+2.83 K | 2.83 K | ||
trait-erc20 |
-2.08 K | -1_928 | 8.44 K | 8_716 |
trait-flipper |
+1.62 K | +22 | 1.62 K | 364 |
trait-incrementer |
+1.73 K | +47 | 1.73 K | 727 |
Link to the run | Last update: Thu Jan 27 10:40:02 CET 2022
I want to clarify, that it is draft PR to show the idea=) I will add documentation and tests when you will agree with the idea(=
Codecov Report
Merging #1017 (2b808cc) into master (2785a90) will increase coverage by
2.74%. The diff coverage is88.48%.
@@ Coverage Diff @@
## master #1017 +/- ##
==========================================
+ Coverage 75.69% 78.44% +2.74%
==========================================
Files 252 253 +1
Lines 9395 9487 +92
==========================================
+ Hits 7112 7442 +330
+ Misses 2283 2045 -238
| Impacted Files | Coverage Δ | |
|---|---|---|
| crates/env/src/api.rs | 40.90% <0.00%> (+4.54%) |
:arrow_up: |
| crates/env/src/backend.rs | 83.87% <ø> (+3.22%) |
:arrow_up: |
| ...tes/env/src/engine/experimental_off_chain/impls.rs | 53.15% <0.00%> (-0.98%) |
:arrow_down: |
| crates/lang/codegen/src/generator/arg_list.rs | 100.00% <ø> (ø) |
|
| crates/lang/codegen/src/generator/mod.rs | 100.00% <ø> (ø) |
|
| crates/lang/src/reflect/dispatch.rs | 70.00% <ø> (+70.00%) |
:arrow_up: |
| crates/env/src/engine/off_chain/impls.rs | 46.55% <20.00%> (+4.17%) |
:arrow_up: |
| .../tests/ui/contract/pass/dispatch-executor-works.rs | 94.59% <94.59%> (ø) |
|
| crates/lang/codegen/src/generator/dispatch.rs | 98.98% <95.12%> (+4.41%) |
:arrow_up: |
| ...rates/env/src/engine/experimental_off_chain/mod.rs | 100.00% <100.00%> (ø) |
|
| ... and 68 more |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
The change reduces the size of each contract except subcontracts of the delegator example. I will check why and how we can improve it too.
The open question is: Are we okay to have a separate static buffer to store input of the contract?
I found that all contracts from workspace contain debug information(stuff related to Debug and Display). It increases the size of all contracts there.
Removing [workspace] decreases the size of each contract. I think it is better to remove it to see the actual size of the contracts.
The change is ready, so I'm waiting for your comments=)
The change affects delegator sub-contract in a positive direction too is saves ~300 bytes.
@xgreenx can you update this for the latest master? I'm curious as to how the size improvements look. Since this is a big change to the dispatch logic I think we should only move forward with it if the savings look good
ink-waterfalldidn't update the sizes=)
Yep, looks like there's a problem (https://github.com/paritytech/ink-waterfall/issues/20).
The job did succeed though, and here are the results (link to run):
| | Δ Optimized Size | Δ Used Gas | Total Optimized Size | Total Used Gas |
|----------------------|------------------|------------|----------------------|----------------|
| `accumulator` | +0.19 K | | 1.62 K | |
| `adder` | -0.12 K | | 2.82 K | |
| `contract-terminate` | -0.12 K | -1 | 1.19 K | 200_777 |
| `contract-transfer` | -0.05 K | +16 | 7.94 K | 1_415 |
| `delegator` | -0.86 K | -1_063 | 9.20 K | 8_447 |
| `dns` | -1.14 K | -1_816 | 9.33 K | 12_472 |
| `erc1155` | -2.88 K | -3_718 | 24.62 K | 44_093 |
| `erc20` | -2.06 K | -1_929 | 8.16 K | 7_828 |
| `erc721` | -2.44 K | +1_264 | 11.26 K | 35_966 |
| `flipper` | +0.04 K | +21 | 1.81 K | 368 |
| `incrementer` | +0.05 K | +23 | 1.69 K | 363 |
| `multisig` | -1.20 K | -6_313 | 27.36 K | 48_583 |
| `proxy` | -0.23 K | -80 | 3.58 K | 2_549 |
| `rand-extension` | -1.07 K | -92 | 4.04 K | 6_562 |
| `subber` | -0.13 K | | 2.83 K | |
| `trait-erc20` | -2.08 K | -1_928 | 8.44 K | 8_716 |
| `trait-flipper` | +0.09 K | +22 | 1.62 K | 364 |
| `trait-incrementer` | +0.11 K | +47 | 1.73 K | 727 |
So the savings are alright, but they're not mindblowing either. I'm on the fence about this given the nature of the changes, but give me some time to try and better understand the new dispatch logic you're proposing before we decide what to do
So the savings are alright, but they're not mindblowing either
The more methods in the contract, the stronger the effect of the change. That contract contains a lot of methods and the change saved 6489 bytes
@xgreenx This has been neglected for a while (I guess mostly by us, sorry).
If you want to revive it please open a new PR so we can a fresh perspective and set of sizes