frequency-chain / frequency

Frequency: A Polkadot Parachain
https://www.frequency.xyz
Apache License 2.0
51 stars 19 forks source link

Benchmarks Issues #2181

Open wilwade opened 1 month ago

wilwade commented 1 month ago

Came across some issues with how we are running benchmarks with @enddynayn

  1. The frequency-bench chain is enabled for the frequency feature, but should only be enabled for runtime-benchmarks feature
  2. When we are running the benchmarks in run_benchmarks.sh it is running with features=runtime-benchmarks,frequency-lint-check. Should it instead be running with features=runtime-benchmarks,frequency?
    • Would this cause issues with non-mainnet pallets?
    • Perhaps it should run this way for release only?
  3. node/service/src/chain_spec/frequency.rs benchmark_mainnet_config should be limited to the runtime-benchmarks feature
wilwade commented 1 month ago

@aramikm can you verify that these are correct actions from your knowledge?

aramikm commented 1 month ago

Looks fine to me. One other thing to consider is that we run benchmarks 2 ways

  1. As release to generate correct weights
  2. As tests (when writing and debugging)

The 2nd use-case should also be taken care of if not already.

aramikm commented 1 month ago

Aside form pallets inclusion or exclusion, using frequency feature or not would have effect if we have feature based logic in our pallets that might alter the weight of benchmarks. I don't think we have anything like that