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

[pallet-revive] change some getter APIs to return value in register #6920

Merged
merged 14 commits into from
Dec 18, 2024

Conversation

xermicus
Copy link
Member

Call data, return data and code sizes can never exceed u32::MAX; they are also not generic. Hence we know that they are guaranteed to always fit into a 64bit register and revive can just zero extend them into a 256bit integer value. Which is slightly more efficient than passing them on the stack.

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
@xermicus xermicus added R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts. labels Dec 17, 2024
@xermicus
Copy link
Member Author

bot fmt

@command-bot
Copy link

command-bot bot commented Dec 17, 2024

@xermicus https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7925662 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 13-ab280ea6-d2cc-4841-9a4d-a9975859cb28 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Dec 17, 2024

@xermicus Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7925662 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7925662/artifacts/download.

@xermicus
Copy link
Member Author

/cmd prdoc --audience runtime_dev --bump major

@xermicus
Copy link
Member Author

/cmd bench --runtime dev --pallet pallet_revive

Copy link

Command "bench --runtime dev --pallet pallet_revive" has started 🚀 See logs here

Copy link

Command "bench --runtime dev --pallet pallet_revive" has finished ✅ See logs here

Subweight results:
File Extrinsic Old New Change [%]
cumulus/pallets/collator-selection/src/weights.rs leave_intent - - ERROR
cumulus/pallets/collator-selection/src/weights.rs new_session - - ERROR
cumulus/pallets/collator-selection/src/weights.rs register_as_candidate - - ERROR
cumulus/pallets/collator-selection/src/weights.rs set_invulnerables - - ERROR
cumulus/pallets/collator-selection/src/weights.rs take_candidate_slot - - ERROR
cumulus/pallets/collator-selection/src/weights.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_ambassador_core.rs promote_fast - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_fellowship_core.rs promote_fast - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
polkadot/runtime/westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmen - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmms - - ERROR
substrate/frame/revive/src/weights.rs seal_return 53.27us 78.21us +46.81
substrate/frame/revive/src/weights.rs seal_input 30.31us 39.55us +30.48
substrate/frame/revive/src/weights.rs seal_balance 4.86us 5.93us +22.10
substrate/frame/revive/src/weights.rs seal_debug_message 190.45us 213.14us +11.91
substrate/frame/revive/src/weights.rs seal_hash_sha2_256 370.16us 389.16us +5.13
substrate/frame/revive/src/weights.rs seal_hash_blake2_256 404.40us 424.80us +5.04
substrate/frame/revive/src/weights.rs seal_contains_transient_storage 2.07us 1.96us -5.10
substrate/frame/revive/src/weights.rs seal_weight_to_fee 1.48us 1.37us -7.48
substrate/frame/revive/src/weights.rs seal_caller_is_origin 422.00ns 374.00ns -11.37
substrate/frame/revive/src/weights.rs seal_minimum_balance 318.00ns 278.00ns -12.58
substrate/frame/revive/src/weights.rs seal_block_number 296.00ns 257.00ns -13.18
substrate/frame/revive/src/weights.rs seal_call_data_size 303.00ns 260.00ns -14.19
substrate/frame/revive/src/weights.rs seal_now 298.00ns 240.00ns -19.46
substrate/frame/revive/src/weights.rs seal_return_data_size 273.00ns Added
Command output:

✅ Successful benchmarks of runtimes/pallets:
-- dev: ['pallet_revive']

substrate/frame/revive/src/benchmarking/mod.rs Outdated Show resolved Hide resolved
xermicus and others added 4 commits December 18, 2024 07:49
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
@xermicus xermicus enabled auto-merge December 18, 2024 08:24
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/12389645649
Failed job name: test-linux-stable

@xermicus xermicus disabled auto-merge December 18, 2024 09:51
@xermicus
Copy link
Member Author

/cmd bench --runtime dev --pallet pallet_revive

Copy link

Command "bench --runtime dev --pallet pallet_revive" has started 🚀 See logs here

Copy link

Command "bench --runtime dev --pallet pallet_revive" has finished ✅ See logs here

Subweight results:
File Extrinsic Old New Change [%]
cumulus/pallets/collator-selection/src/weights.rs leave_intent - - ERROR
cumulus/pallets/collator-selection/src/weights.rs new_session - - ERROR
cumulus/pallets/collator-selection/src/weights.rs register_as_candidate - - ERROR
cumulus/pallets/collator-selection/src/weights.rs set_invulnerables - - ERROR
cumulus/pallets/collator-selection/src/weights.rs take_candidate_slot - - ERROR
cumulus/pallets/collator-selection/src/weights.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_ambassador_core.rs promote_fast - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_fellowship_core.rs promote_fast - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
polkadot/runtime/westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmen - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmms - - ERROR
substrate/frame/revive/src/weights.rs noop_host_fn 256.69us 273.78us +6.66
substrate/frame/revive/src/weights.rs seal_value_transferred 304.00ns 323.00ns +6.25
substrate/frame/revive/src/weights.rs seal_call_data_load 305.00ns 322.00ns +5.57
substrate/frame/revive/src/weights.rs rollback_transient_storage 1.18us 1.24us +5.36
substrate/frame/revive/src/weights.rs seal_minimum_balance 292.00ns 275.00ns -5.82
substrate/frame/revive/src/weights.rs seal_call_data_size 288.00ns 271.00ns -5.90
substrate/frame/revive/src/weights.rs seal_weight_to_fee 1.55us 1.39us -10.34
substrate/frame/revive/src/weights.rs seal_caller_is_root 331.00ns 296.00ns -10.57
substrate/frame/revive/src/weights.rs seal_return 62.60us 55.84us -10.79
substrate/frame/revive/src/weights.rs seal_copy_to_contract 62.69us 54.40us -13.23
substrate/frame/revive/src/weights.rs seal_ref_time_left 306.00ns 265.00ns -13.40
substrate/frame/revive/src/weights.rs seal_call_data_copy 39.16us 29.91us -23.61
substrate/frame/revive/src/weights.rs seal_return_data_size 268.00ns Added
Command output:

✅ Successful benchmarks of runtimes/pallets:
-- dev: ['pallet_revive']

@xermicus xermicus enabled auto-merge December 18, 2024 14:32
@xermicus xermicus added this pull request to the merge queue Dec 18, 2024
Merged via the queue into master with commit 53f6473 Dec 18, 2024
195 of 201 checks passed
@xermicus xermicus deleted the cl/register-ret branch December 18, 2024 15:45
dudo50 pushed a commit to paraspell-research/polkadot-sdk that referenced this pull request Jan 4, 2025
…aritytech#6920)

Call data, return data and code sizes can never exceed `u32::MAX`; they
are also not generic. Hence we know that they are guaranteed to always
fit into a 64bit register and `revive` can just zero extend them into a
256bit integer value. Which is slightly more efficient than passing them
on the stack.

---------

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
Co-authored-by: command-bot <>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants