Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

contracts: Use proof_size from benchmarks #13268

Merged
merged 23 commits into from
Feb 15, 2023
Merged

Conversation

athei
Copy link
Member

@athei athei commented Jan 29, 2023

We remove our ad-hoc injected proof_size and replace it by the benchmarking results. We also refactor wasm::code_cache::store to never load the code from storage if it was supplied via the extrinsic.

We use #[pov_mode = Measure] everywhere because the max encoded length of the used storage items is usually much bigger than the average. All benchmarks are already written in a way that the proof_size scales with the components just as the ref_time does.

We also fixed the deletion queue which wasn't honouring the proof_size. In the process we added a new associated function: Weight::checked_div_per_component.

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jan 29, 2023
@athei
Copy link
Member Author

athei commented Jan 29, 2023

/cmd queue -c bench $ pallet runtime pallet_contracts

@athei athei added B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Jan 29, 2023
@athei
Copy link
Member Author

athei commented Jan 29, 2023

/cmd queue -c bench $ pallet dev pallet_contracts

@athei athei marked this pull request as ready for review January 30, 2023 17:31
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jan 30, 2023
.saturating_add(T::DbWeight::get().reads(6_u64))
.saturating_add(T::DbWeight::get().writes(4_u64))
.saturating_add(Weight::from_proof_size(5).saturating_mul(c.into()))
Copy link
Member Author

Choose a reason for hiding this comment

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

Oof. 5 bytes per byte of code. That is bad. It is probably due to our instrumentation worst case. We really need to get rid of that and move that into wasmi.

@athei athei requested review from agryaznov and xermicus February 1, 2023 12:55
frame/contracts/src/schedule.rs Outdated Show resolved Hide resolved
frame/contracts/src/wasm/code_cache.rs Show resolved Hide resolved
Copy link
Member

@xermicus xermicus left a comment

Choose a reason for hiding this comment

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

LGTM

@athei
Copy link
Member Author

athei commented Feb 2, 2023

bot rebase

@paritytech paritytech deleted a comment from command-bot bot Feb 14, 2023
@athei
Copy link
Member Author

athei commented Feb 14, 2023

bot bench -v PIPELINE_SCRIPTS_REF=bm-fallback $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Feb 14, 2023

@athei https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2392639 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 18-df94ff54-494a-48dd-b3a4-4e15bb294ee3 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Feb 14, 2023

@athei Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2392639 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2392639/artifacts/download.

frame/contracts/src/weights.rs Outdated Show resolved Hide resolved
@athei
Copy link
Member Author

athei commented Feb 15, 2023

bot bench -v PIPELINE_SCRIPTS_REF=bm-fallback $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Feb 15, 2023

@athei https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2398023 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 28-09f09183-f16f-4bc5-85da-39a2e801b835 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Feb 15, 2023

@athei Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2398023 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2398023/artifacts/download.

@athei athei merged commit 7732f88 into master Feb 15, 2023
@athei athei deleted the at/contracts-proof-size branch February 15, 2023 21:56
rossbulat pushed a commit that referenced this pull request Feb 19, 2023
* Avoid reading contract code when it is supplied in the extrinsic

* Remove custom proof size injection from schedule

* Set benchmarks pov_mode to Measure

* Reduce overestimation of code size on re-instrument

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Do not override proof size from benchmark

* Do not charge proof size for basic block

* Incrase gas limit for tests

* Fix deletion queue to also use `proof_size`

* Fix tests

* Update frame/contracts/src/schedule.rs

Co-authored-by: Cyrill Leutwiler <[email protected]>

* Fix wrong schedule macro invocations

* Remove stale docs

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Handle zero components

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Fix instruction weight

---------

Co-authored-by: command-bot <>
Co-authored-by: Cyrill Leutwiler <[email protected]>
rossbulat pushed a commit that referenced this pull request Feb 19, 2023
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* Avoid reading contract code when it is supplied in the extrinsic

* Remove custom proof size injection from schedule

* Set benchmarks pov_mode to Measure

* Reduce overestimation of code size on re-instrument

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Do not override proof size from benchmark

* Do not charge proof size for basic block

* Incrase gas limit for tests

* Fix deletion queue to also use `proof_size`

* Fix tests

* Update frame/contracts/src/schedule.rs

Co-authored-by: Cyrill Leutwiler <[email protected]>

* Fix wrong schedule macro invocations

* Remove stale docs

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Handle zero components

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Fix instruction weight

---------

Co-authored-by: command-bot <>
Co-authored-by: Cyrill Leutwiler <[email protected]>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Avoid reading contract code when it is supplied in the extrinsic

* Remove custom proof size injection from schedule

* Set benchmarks pov_mode to Measure

* Reduce overestimation of code size on re-instrument

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Do not override proof size from benchmark

* Do not charge proof size for basic block

* Incrase gas limit for tests

* Fix deletion queue to also use `proof_size`

* Fix tests

* Update frame/contracts/src/schedule.rs

Co-authored-by: Cyrill Leutwiler <[email protected]>

* Fix wrong schedule macro invocations

* Remove stale docs

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Handle zero components

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Fix instruction weight

---------

Co-authored-by: command-bot <>
Co-authored-by: Cyrill Leutwiler <[email protected]>
Ank4n pushed a commit that referenced this pull request Feb 28, 2023
* Avoid reading contract code when it is supplied in the extrinsic

* Remove custom proof size injection from schedule

* Set benchmarks pov_mode to Measure

* Reduce overestimation of code size on re-instrument

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Do not override proof size from benchmark

* Do not charge proof size for basic block

* Incrase gas limit for tests

* Fix deletion queue to also use `proof_size`

* Fix tests

* Update frame/contracts/src/schedule.rs

Co-authored-by: Cyrill Leutwiler <[email protected]>

* Fix wrong schedule macro invocations

* Remove stale docs

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Handle zero components

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Fix instruction weight

---------

Co-authored-by: command-bot <>
Co-authored-by: Cyrill Leutwiler <[email protected]>
@jakoblell jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Mar 2, 2023
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
* Avoid reading contract code when it is supplied in the extrinsic

* Remove custom proof size injection from schedule

* Set benchmarks pov_mode to Measure

* Reduce overestimation of code size on re-instrument

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Do not override proof size from benchmark

* Do not charge proof size for basic block

* Incrase gas limit for tests

* Fix deletion queue to also use `proof_size`

* Fix tests

* Update frame/contracts/src/schedule.rs

Co-authored-by: Cyrill Leutwiler <[email protected]>

* Fix wrong schedule macro invocations

* Remove stale docs

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Handle zero components

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Fix instruction weight

---------

Co-authored-by: command-bot <>
Co-authored-by: Cyrill Leutwiler <[email protected]>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Avoid reading contract code when it is supplied in the extrinsic

* Remove custom proof size injection from schedule

* Set benchmarks pov_mode to Measure

* Reduce overestimation of code size on re-instrument

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Do not override proof size from benchmark

* Do not charge proof size for basic block

* Incrase gas limit for tests

* Fix deletion queue to also use `proof_size`

* Fix tests

* Update frame/contracts/src/schedule.rs

Co-authored-by: Cyrill Leutwiler <[email protected]>

* Fix wrong schedule macro invocations

* Remove stale docs

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Handle zero components

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Fix instruction weight

---------

Co-authored-by: command-bot <>
Co-authored-by: Cyrill Leutwiler <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

7 participants