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

[contracts] Fix *storage_per_kb benchmarks #11756

Closed
wants to merge 10 commits into from
Closed

Conversation

agryaznov
Copy link
Contributor

As @athei noticed in #11501, *storage_per_kb benchmarks include a read/write component per n, which is not right.

By this PR, storage keys generated in those benchmarks are whitelisted in DB in order to fix that.

@agryaznov agryaznov added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jun 29, 2022
@agryaznov agryaznov requested a review from athei as a code owner June 29, 2022 20:57
@paritytech paritytech deleted a comment from command-bot bot Jul 7, 2022
@paritytech paritytech deleted a comment from command-bot bot Jul 7, 2022
@joao-paulo-parity
Copy link
Contributor

/cmd queue -c bench-bot $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Jul 7, 2022

@joao-paulo-parity https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1664124 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.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 /cmd cancel 6-f3f44f44-c298-4d5b-bfa6-3e00dafc1f63 to cancel this command or /cmd cancel to cancel all commands in this pull request.

@paritytech paritytech deleted a comment from radupopa2010 Jul 7, 2022
@command-bot
Copy link

command-bot bot commented Jul 7, 2022

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

@joao-paulo-parity
Copy link
Contributor

Last job timed out due to a problem in the bot, but it should be fixed now. Let's try again:

/cmd queue -c bench-bot $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Jul 7, 2022

@joao-paulo-parity https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1664368 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.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 /cmd cancel 1-739e1a08-e613-4377-85db-346415d59192 to cancel this command or /cmd cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jul 7, 2022

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

@athei
Copy link
Member

athei commented Jul 14, 2022

This didn't really removed the reads per n from the benchmark results. The reason is because the keys that are used by the contract are hashed again before they are actually used. This is why the whitelist doesn't apply.

@radupopa2010
Copy link
Contributor

@joao-paulo-parity
Copy link
Contributor

joao-paulo-parity commented Jul 15, 2022

@athei re #11756 (comment), as Oliver noticed in #11756 (comment) the benchmarks were using the wrong machine by accident, but it seems like this was fixed in https://github.com/paritytech/ci_cd/issues/518#issuecomment-1184634791. Sorry about that! Let's try it again:

/cmd queue -c bench-bot $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Jul 15, 2022

@joao-paulo-parity https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1677829 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.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 /cmd cancel 7-7f1f8015-8c0c-4f35-b976-8d8e192729d5 to cancel this command or /cmd cancel to cancel all commands in this pull request.

@paritytech paritytech deleted a comment from command-bot bot Jul 15, 2022
@paritytech paritytech deleted a comment from command-bot bot Jul 15, 2022
@command-bot
Copy link

command-bot bot commented Jul 15, 2022

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

@athei
Copy link
Member

athei commented Jul 26, 2022

/cmd queue -c bench-bot $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Jul 26, 2022

@athei https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1700666 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.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 /cmd cancel 27-230f28d8-bc73-4fb4-9c9d-36a45e395dd2 to cancel this command or /cmd cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jul 26, 2022

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

@athei
Copy link
Member

athei commented Jul 28, 2022

Ok this also didn't work. We will just live with the slight overestimation.

@athei athei closed this Jul 28, 2022
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. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants