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

Update pallet-domains benchmarking #2592

Merged
merged 7 commits into from
Mar 15, 2024
Merged

Update pallet-domains benchmarking #2592

merged 7 commits into from
Mar 15, 2024

Conversation

NingLin-P
Copy link
Member

This PR updates the pallet-domains benchmarking according to the changes that happened since the last benchmark, e.g. staking 1.1, challenge period update, lazily processing bad ER, etc.

Some notable changes:

  • Add benchmark for submit_fraud_proof and the extrinsics introduced in staking 1.1
  • The MaxPendingStakingOperation is set to 512 and the meaning is changed to the number of operators who has deposit/withdrawls or newly register in the current epoch
    • This is because do_finalize_domain_epoch_staking will need to update the storage item for every operator who has deposit/withdrawls in the current epoch thus the number needs to be bounded.
  • More accurate and fine-grained weight, the actual weight returned is calculated according to the actual number of nominators/operators being processed

Also, because we don't have an explicit upper bound for the number of bundles that can be included in a block, this PR hypocritically uses MAX_BUNLDE_PER_BLOCK = 100 to calculate the worst situation weight but we will later return the actual weight according to the executing details, which work as follow:

  1. Deduct the worst situation weight from the available block weight
    • Skip the extrinsic if the worst situation weight is greater than the available block weight
  2. Execute the extrinsic, and collect executing details (e.g. the number of nominators/operators being processed) to calculate the actual weight
  3. Refund the difference between the worst situation weight and the actual weight to the available block weight

TL;DR Even with staking 1.1 there are still 2 bottlenecks exists: do_finalize_slashed_operators and do_finalize_domain_epoch_staking

According to the benchmark result, the worst situation weight of submit_bundle is Weight { ref_time: 197_468_758_228, proof_size: 5524645 } (197ms), the majority of the weight is contributed by the worst situation weight of epoch transition Weight { ref_time: 174_790_184_468, proof_size: 4825721 } (174 ms) which consist of:

  • operator_take_reward_tax_and_stake: Weight { ref_time: 6_169_916_280, proof_size: 273782 } (6ms)
  • do_finalize_slashed_operators: Weight { ref_time: 94_643_292_160, proof_size: 1850909 } (94ms)
    • Slashing operator need to iterate over all the nominators, this is weight for iterating 256 nominators, but in the worst situation assume 100 operator submitted the same bad ER and each operator have 256 nominators the weight will be Weight { ref_time: 6_771_133_216_000, proof_size: 134222621 } (> 6s)
  • do_finalize_domain_epoch_staking: Weight { ref_time: 73_976_976_028, proof_size: 2701030 } (73ms)
    • Within do_finalize_domain_epoch_staking we need to read all of the operators of the domain (the number of operator is unbounded but we hypocritically use MaxPendingStakingOperation = 512 in the benchmark) and write to operator who has deposit/withdrawls in the current epoch (the number is bounded by MaxPendingStakingOperation = 512)

Code contributor checklist:

…rrent epoch

Also the MaxPendingStakingOperation is set to 512 in this commit, which means
there can be at most 512 number of operator has deposit/withdraw happen in the
current epoch

Signed-off-by: linning <[email protected]>
… get/set instead of try_mutate

When using try_mutate the short-circuit for operator who has no deposits/withdrawls/rewards
is not working because try_mutate will write back storage item as Ok() is returned

Signed-off-by: linning <[email protected]>
These info will be used later to calculate more accurate weight

Signed-off-by: linning <[email protected]>
// Measured: `1609`
// Estimated: `7549`
// Minimum execution time: 82_000_000 picoseconds.
Weight::from_parts(86_000_000, 7549)
Copy link
Member

Choose a reason for hiding this comment

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

Are these worst case estimates? or is this fixed cost for farmer because the rest of execution happens on operator side?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are these worst case estimates?

Bad naming again 🤦‍♂️ it should be basic_submit_bundle it is the worst case estimation for the basic flow of submit_bundle extrinsic, it covers:

  • Add the ER into the block tree
  • Prune the oldest (confirmed) ER
  • Increase HeadReciptNumber and HeadDomainNumber, and update SuccessfulBundles, ExecutionInbox, InboxedBundleAuthor
  • etc

also the database read/write weight in the next 2 line should be counted.

For other work that will be done within the submit_bundle extrinsic like

  • Prune bad ER and slash submitter (as lazily process bad ER descendant)
  • Confirm ER, refund storage fee, reward submitter and slash invalid bundle submitter
  • Epoch transition

They are benchmarked separately, they are counted for the max weight of submit_bundle but they are only added to the actual weight if they did happen. See fn max_submit_bundle_weight

or is this fixed cost for farmer because the rest of execution happens on operator side?

Right, all of the weight of pallet-domains is for the farmer, the weight of executing the domain tx is not counted here since the farmer won't execute the domain tx.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so for a "basic" bundle the weight for farmer is this 86 000 000 + 11*read weight + 13*write weight
Using these parity db weights 86 000 000 + 11*8 000 000+ 13*50 000 000 = 824 000 000 which means we could fit up to 1820 basic bundles into a consensus block from farmers perspective (assuming each bundle fits limits for its domain execution). Does that sound right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Almost, I think we also need to take the weight for confirm_domain_block into count, which will happen on every consensus block that contains a bundle after the first challenge period is passed. But it should only happen once in a consensus block, i.e. if the consensus block contains multiple bundles only the first bundle has confirm_domain_block.

For other things, handle_bad_receipt only happen when there is FP, and max_staking_epoch_transition happen once 100 domain blocks (StakeEpochDuration).

vedhavyas
vedhavyas previously approved these changes Mar 15, 2024
@NingLin-P NingLin-P added this pull request to the merge queue Mar 15, 2024
Merged via the queue into main with commit 0905b9d Mar 15, 2024
11 checks passed
@NingLin-P NingLin-P deleted the submit-fp-benchmark branch March 15, 2024 13:53
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.

3 participants