Skip to content
Snippets Groups Projects
  • ordian's avatar
    inclusion: bench `enact_candidate` weight (#5270) · ddd58c15
    ordian authored
    On top of #5082.
    
    ## Background
    
    Previously, before #3479, we would
    [include](https://github.com/paritytech/polkadot-sdk/blame/75074952/polkadot/runtime/parachains/src/builder.rs#L508C12-L508C44)
    the cost enacting the candidate into the cost of processing a single
    bitfield.
    [Now](https://github.com/paritytech/polkadot-sdk/blame/dd48544a/polkadot/runtime/parachains/src/builder.rs#L529)
    it is different, although the benchmarks seems to be not-up-to date.
    Including the cost of enacting a candidate into a processing a single
    bitfield cost was incorrect, since we multiple that by the number of
    bitfields we have. Instead, we should separate calculate the cost of
    processing a single bitfield without enactment, and multiple the cost of
    enactment by the actual number of processed candidates (which is limited
    by the number cores, not validators).
    
    ## Bench
    
    Previously, the weight of `enact_candidate` was calculated manually
    (without a benchmark) and then neglected:
    https://github.com/paritytech/polkadot-sdk/blob/dd48544a
    
    /polkadot/runtime/parachains/src/inclusion/mod.rs#L584
    
    In this PR, we have a benchmark for it and it's based on the number of
    ump and sent hrmp messages as well as whether the candidate has a
    runtime upgrade (new_validation_code).
    The differences from the previous attempt
    https://github.com/paritytech/polkadot/pull/6929 are that
    * we don't include the cost of enactment into the cost of processing a
    backed candidate.
    The reason for it is that enactment happens not in the same block as
    backing (typically the next one), since we process bitfields before
    backing votes.
    * we don't take into account the size of the runtime upgrade, the
    benchmark weight doesn't seem to depend much on it, but rather whether
    there was one or not.
    
    Similarly to the previous attempt, we don't account for dmp messages
    (fixed cost). Also we don't account properly for received hrmp messages
    (hrmp_watermark) because the cost of it depends on the runtime state and
    can't be statically deduced in the benchmark (unless we pass the
    information about channels as benchmark u32 arguments).
    
    The total weight cost of processing a parainherent now includes the cost
    of enactment of each candidate, but we don't do filtering based on that
    (because we enact after processing bitfields and making other changes to
    the storage).
    
    ## Numbers
    
    ```
    Reads = 7 + (0 * u) + (3 * h) + (8 * c)
    Writes = 10 + (1 * u) + (3 * h) + (7 * c)
    ```
    In addition, there is a fixed cost of a few of ms (!) per candidate. 
    
    This might result a full block slightly overflowing its weight with 200
    enacted candidates, which in turn could prevent non-mandatory
    transactions from being included in a block.
    
    Given our modest limits on max ump and hrmp messages:
    ```
      maxUpwardMessageNumPerCandidate: 16
      hrmpMaxMessageNumPerCandidate: 10
    ```
    and the fact that runtime upgrades are can't happen very frequently
    (`validation_upgrade_cooldown`), we might only go over the limits in
    case of many disputes.
    
    TODOs:
    - [x] Fix the overweight test
    - [x] Generate the weights for Westend and Rococo
    - [x] PRDoc
    
    ---------
    
    Co-authored-by: command-bot <>
    Co-authored-by: default avatarAlin Dima <alin@parity.io>
    Unverified
    ddd58c15
Code owners
Assign users and groups as approvers for specific file changes. Learn more.