Update production runtime weight #3578

NingLin-P · 2025-06-12T23:54:24Z

This PR consists of changes:

Update the pallet weight from pallet-specific to runtime-specific.
- In the former, each pallet has its own weight and is used across different runtimes. In the latter, each production runtime generates and uses its own weight for the pallet, because given different reference machine and pallet parameters in the runtime, the weight may be different.
Add a script to run benchmark for all the pallets (including the upstream pallet) that are used by our runtimes.
Generate weight on our reference machine AMD Ryzen 5 3600 6-Core Processor, and apply the resulting weight to our production runtimes.
Fix broken benchmarks, add benchmarks, and some minor refactoring.

Please review commit by commit.

Code contributor checklist:

I have read, understood and followed contributing guide

Signed-off-by: linning <[email protected]>

This benchmark is update long time ago but its weight is not Signed-off-by: linning <[email protected]>

Signed-off-by: linning <[email protected]>

All weight files are generated by running './runtime_benchmark.sh' Signed-off-by: linning <[email protected]>

Signed-off-by: linning <[email protected]>

teor2345

Thank you for all this work! Looks good, I don't think there are any blockers, but we will need to open some follow-up tickets.

teor2345 · 2025-06-13T00:06:54Z

crates/pallet-domains/src/lib.rs

 use crate::domain_registry::{DomainConfig, Error as DomainRegistryError};
 use crate::runtime_registry::into_complete_raw_genesis;
 use crate::staking::OperatorStatus;
 #[cfg(feature = "runtime-benchmarks")]


PR #3557 adds a benchmark for the balance transfer check extension. How are you going to merge those changes in this set of benchmarks? Do we need to re-run the script after it merges?

If we're soon going to remove that extension, is it worth having the weights for it in the runtime?

Partially answered in https://github.com/autonomys/subspace/pull/3578/files#r2143887125.

If we're soon going to remove that extension, is it worth having the weights for it in the runtime?

Not really removing the extension, I think we just change the enable_transfer from false to true, the extension is still there and we still need to do this check for every extrinsic.

teor2345 · 2025-06-13T00:09:44Z

domains/runtime/evm/src/lib.rs

 #![feature(variant_count)]
 #![cfg_attr(not(feature = "std"), no_std)]
 // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256.
 #![recursion_limit = "256"]


Same questions, but for the EVM contract creation check extension in PR #3554. What do we need to do after that PR gets approved and merged?

if this PR lands before that PR, then you can update the weights in your PR using the reference machine

The main goal of this PR is to introduce the infra, the new weight structure, and the script. After it is merged, we still need to update the runtime weight regularly, especially before making a new runtime release (which indicates the runtime code changed thus we should re-benchmark), and with script this process should be much easier.

I think we should merge it as is and do another round of benchmark to update the weight before the official mainnet runtime release.

teor2345 · 2025-06-13T00:29:04Z

crates/subspace-runtime/src/weights/frame_system.rs

+			.saturating_add(Weight::from_parts(1_089, 0).saturating_mul(b.into()))
+	}
+	/// Storage: UNKNOWN KEY `0x3a686561707061676573` (r:0 w:1)
+	/// Proof: UNKNOWN KEY `0x3a686561707061676573` (r:0 w:1)


This "UNKNOWN KEY" comment looks like an error, does it impact the result of the benchmark?

I think this represents the direct storage access with raw storage key in frame-system, this also exists in the upstream weight and should not affect the benchmark result.

teor2345 · 2025-06-13T00:42:13Z

crates/subspace-runtime/src/weights/pallet_domains.rs

+	/// Proof: `Domains::NextRuntimeId` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	/// Storage: `Domains::RuntimeRegistry` (r:0 w:1)
+	/// Proof: `Domains::RuntimeRegistry` (`max_values`: None, `max_size`: None, mode: `Measured`)
+	fn register_domain_runtime() -> Weight {


Should the runtime registration weight depend on the size of the domain runtime?
Only a blocker for allowing anyone to upload a domain runtime, but we should have a ticket and a TODO.

I also have the same question about instantiating a domain and upgrading a runtime.

Should the runtime registration weight depend on the size of the domain runtime?

This benchmark running with a production domain runtime (though it may out of date now), but I don't think the substrate benchmark is smart enough to calculate the weight based the size, the same also happen to db read/write where no matter what size of the data it read/write it take the same weight.

Only a blocker for allowing anyone to upload a domain runtime, but we should have a ticket and a TODO.

This is not supported since attacker can inject malicious code into the runtime to do whatever they want, like mint coins out of thin air.

Updating the WASM we're using for this benchmark is now #3581

teor2345 · 2025-06-13T00:45:29Z

crates/subspace-runtime/src/weights/pallet_domains.rs

+	}
+	/// Storage: `Domains::DomainRegistry` (r:1 w:1)
+	/// Proof: `Domains::DomainRegistry` (`max_values`: None, `max_size`: None, mode: `Measured`)
+	fn update_domain_operator_allow_list() -> Weight {


Should this weight depend on the number of operators?
Similar question for the EVM contract creation allow list call, which appears to be missing a benchmark/weight.

Ideally, it should, but the substrate benchmark system is not that smart, no matter what size the list is, the benchmark only counts one db write, and perhaps with some minor constant weight added as the size increases.

crates/subspace-runtime/src/weights/pallet_messenger.rs

teor2345 · 2025-06-13T01:04:30Z

runtime-benchmark.sh

@@ -0,0 +1,81 @@
+#!/bin/bash -e


Next time can you please put autogenerated code in a separate commit from the script?
It makes the script much easier to find and review.

teor2345 · 2025-06-13T01:07:53Z

runtime-benchmark.sh

@@ -0,0 +1,81 @@
+#!/bin/bash -e
+
+cargo build -r --bin subspace-node --features runtime-benchmarks


Most of our builds use --profile production, which can be slightly more performant than release. It's ok to benchmark with the less efficient profile, but let's leave a comment explaining why that's ok?

Nitpicks:

It might be helpful to define common options at the top of the script, so the build profile and features are the same for every command

I usually spell out command-line options in scripts, so it is easier to read and review them. Sometimes short options can hide subtle bugs.

cargo build --release --bin subspace-node --features runtime-benchmarks

runtime-benchmark.sh

teor2345 · 2025-06-13T01:32:59Z

runtime-benchmark.sh

+
+cargo build -r -p subspace-runtime --features runtime-benchmarks
+subspace_runtime_pallets=(
+    "frame_system"


How are we going to keep these pallet lists in sync with the ones in the runtimes?

for now, keep track if any additions or removal 🤷🏼‍♂️
If we can improve it, we can do it down the line

Hw are we going to keep these pallet lists in sync with the ones in the runtimes?

Unfortunately, AFAIK there is no such API that we can use, thus we have to do it manually.

We can use command-line text processing in the shell script, here is a ticket:
#3582

teor2345 · 2025-06-13T01:48:05Z

Looks like we're almost finished #1520, @NingLin-P can you add some docs on the benchmark hardware and process?
Then we can close that ticket.
(And just focus on filling in a few benchmark gaps.)

vedhavyas

Small nit on moving fixture tests under features.
Rest looks good to me

vedhavyas · 2025-06-13T09:23:23Z

domains/pallets/messenger/src/tests.rs

        .unwrap();
    }
+
+    #[test]


ideal to put them behind benchmarks features so that the fixture is not generated everytime

They are inside mod benchmark_fixtures_gen, which is behind runtime-benchmarks, just like other fixture generator tests.

vedhavyas · 2025-06-13T09:25:00Z

domains/runtime/evm/src/lib.rs

 #![feature(variant_count)]
 #![cfg_attr(not(feature = "std"), no_std)]
 // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256.
 #![recursion_limit = "256"]


if this PR lands before that PR, then you can update the weights in your PR using the reference machine

vedhavyas · 2025-06-13T09:27:02Z

runtime-benchmark.sh

+
+cargo build -r -p subspace-runtime --features runtime-benchmarks
+subspace_runtime_pallets=(
+    "frame_system"


for now, keep track if any additions or removal 🤷🏼‍♂️
If we can improve it, we can do it down the line

Signed-off-by: linning <[email protected]>

NingLin-P added 10 commits June 13, 2025 06:52

Fix broken benchmarks in pallet-transporter and pallet-messenger

d38f0ca

Signed-off-by: linning <[email protected]>

Update weight of the unlock_funds benchmark in pallet-domains

6f4464f

This benchmark is update long time ago but its weight is not Signed-off-by: linning <[email protected]>

Add benchmark for XDM between domains in the pallet-messenger extension

87e0e4b

Signed-off-by: linning <[email protected]>

Update pallet-messenger extension SubstrateWeight type

ee7ba9f

Signed-off-by: linning <[email protected]>

Export pallet WeightInfo trait

c977079

Signed-off-by: linning <[email protected]>

Add more (mostly upstream) pallets to runtime benchmark list

86dd424

Signed-off-by: linning <[email protected]>

Init weights files structure in the runtime

a6e2670

Signed-off-by: linning <[email protected]>

Add and run scripte to generate weight files

1a273f9

All weight files are generated by running './runtime_benchmark.sh' Signed-off-by: linning <[email protected]>

Recorrect the WeightInfo trait for the extension weight files

df64e4a

Signed-off-by: linning <[email protected]>

Apply weight to the production runtime

3c79afe

Signed-off-by: linning <[email protected]>

NingLin-P requested a review from teor2345 June 12, 2025 23:54

NingLin-P requested review from nazar-pc and vedhavyas as code owners June 12, 2025 23:54

teor2345 previously approved these changes Jun 13, 2025

View reviewed changes

vedhavyas reviewed Jun 13, 2025

View reviewed changes

NingLin-P added 2 commits June 14, 2025 00:54

Fix clippy & update runtime-benchmark.sh

4933789

Signed-off-by: linning <[email protected]>

Merge branch 'main' of github.com:subspace/subspace into update-weight

11623ab

NingLin-P dismissed teor2345’s stale review via 11623ab June 13, 2025 17:02

vedhavyas approved these changes Jun 13, 2025

View reviewed changes

vedhavyas added this pull request to the merge queue Jun 13, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2025

vedhavyas added this pull request to the merge queue Jun 15, 2025

Merged via the queue into main with commit fd064e1 Jun 15, 2025
12 checks passed

vedhavyas deleted the update-weight branch June 15, 2025 17:35

This was referenced Jun 16, 2025

Use recent runtime WASM for runtime code upload benchmarks #3581

Open

Keep benchmark script pallet lists in sync with the runtime #3582

Closed

		@@ -0,0 +1,81 @@
		#!/bin/bash -e

		cargo build -r --bin subspace-node --features runtime-benchmarks

Update production runtime weight #3578

Update production runtime weight #3578

Uh oh!

Conversation

NingLin-P commented Jun 12, 2025

Code contributor checklist:

Uh oh!

teor2345 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teor2345 Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teor2345 Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teor2345 commented Jun 13, 2025

Uh oh!

vedhavyas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

teor2345 Jun 13, 2025 •

edited

Loading

teor2345 Jun 13, 2025 •

edited

Loading