polkadot-sdk
polkadot-sdk copied to clipboard
Contracts: Rework host fn benchmarks
fix https://github.com/paritytech/polkadot-sdk/issues/4163
This PR does the following: Update to pallet-contracts-proc-macro:
- Parse #[cfg] so we can add a dummy noop host function for benchmark.
- Generate BenchEnv::<host_fn> so we can call host functions directly in the benchmark.
- Add the weight of the noop host function before calling the host function itself
Update benchmarks:
- Update all host function benchmark, a host function benchmark now simply call the host function, instead of invoking the function n times from within a contract.
- Refactor RuntimeCosts & Schedule, for most host functions, we can now use the generated weight function directly instead of computing the diff with the cost! macro
// Before
#[benchmark(pov_mode = Measured)]
fn seal_input(r: Linear<0, API_BENCHMARK_RUNS>) {
let code = WasmModule::<T>::from(ModuleDefinition {
memory: Some(ImportedMemory::max::<T>()),
imported_functions: vec![ImportedFunction {
module: "seal0",
name: "seal_input",
params: vec![ValueType::I32, ValueType::I32],
return_type: None,
}],
data_segments: vec![DataSegment { offset: 0, value: 0u32.to_le_bytes().to_vec() }],
call_body: Some(body::repeated(
r,
&[
Instruction::I32Const(4), // ptr where to store output
Instruction::I32Const(0), // ptr to length
Instruction::Call(0),
],
)),
..Default::default()
});
call_builder!(func, code);
let res;
#[block]
{
res = func.call();
}
assert_eq!(res.did_revert(), false);
}
// After
fn seal_input(n: Linear<0, { code::max_pages::<T>() * 64 * 1024 - 4 }>) {
let mut setup = CallSetup::<T>::default();
let (mut ext, _) = setup.ext();
let mut runtime = crate::wasm::Runtime::new(&mut ext, vec![42u8; n as usize]);
let mut memory = memory!(n.to_le_bytes(), vec![0u8; n as usize],);
let result;
#[block]
{
result = BenchEnv::seal0_input(&mut runtime, &mut memory, 4, 0)
}
assert_ok!(result);
assert_eq!(&memory[4..], &vec![42u8; n as usize]);
}
bot bench substrate-pallet --pallet=pallet_contracts
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6079780 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
. 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 2-7dfaefeb-354e-4724-9d02-8a1bc5c47e45
to cancel this command or bot cancel
to cancel all commands in this pull request.
@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6079780 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6079780/artifacts/download.
bot bench substrate-pallet --pallet=pallet_contracts
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6082811 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
. 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 7-b73e1a52-9d8a-4f41-9b1d-2ca6f1dd95a3
to cancel this command or bot cancel
to cancel all commands in this pull request.
@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6082811 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6082811/artifacts/download.
bot bench substrate-pallet --pallet=pallet_contracts
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6083656 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
. 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 1-b3fd50f2-1f85-4269-a966-591e00f68cf2
to cancel this command or bot cancel
to cancel all commands in this pull request.
@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6083656 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6083656/artifacts/download.
At first glance it looks like weights go down. But when looking at the formular we can see that the weights go up for many host functions:
https://weights.tasty.limo/compare?unit=weight&ignore_errors=true&threshold=10&method=asymptotic&repo=polkadot-sdk&old=master&new=pg%2Frework-host-benchs&path_pattern=substrate%2Fframe%2Fcontracts%2Fsrc%2Fweights.rs%2Cpolkadot%2Fruntime%2F*%2Fsrc%2Fweights%2F**%2F*.rs%2Cpolkadot%2Fbridges%2Fmodules%2F*%2Fsrc%2Fweights.rs%2Ccumulus%2F**%2Fweights%2F*.rs%2Ccumulus%2F**%2Fweights%2Fxcm%2F*.rs%2Ccumulus%2F**%2Fsrc%2Fweights.rs
Before it was 280k and not it is 680k. They kind of should go down because no we are running the host functions directly.
Not sure if it should go down, but it should be roughly equal for sure. To be correct you would need to add as well the noop_host_fn weight which adds another 70k, since the previous measured time was (fn time + base cost of calling a host fn) repeated r times.
Yes it should be roughly the same. Even slightly less after the rework because we still add the noop_host_fn
onto the new weight as you pointed out.
Could be attributed to caching behaviour. Before we ran the same code over and over again. That could make each of them faster.
Yes exactly for host fn where there is io, there are some trie node cached that amortized the cost but for other like gas_left where there are no PoV this must be something else 🤔
For the benchmarks which touch storage (
seal_call,
seal_instantiate,
seal_delegate_call`, ...) we need to whitelist the callers address and probably other keys.
Added that to the host fn benchmark setup. Storage access for the contract account_id and ContractInfo (that is touched when we add / pop a frame) are already taken into account with the base call extrinsic benchmark.
Everything else should be fine (will double check once the benchmark are updated)
I have also removed from the kitchensink in this PR #4489 the dynamic parameters that should not be used with pallet-contracts until there is a proper way to not double count them in the host fn benchmarks
// Whitelist contract account, as it is already accounted for in the call benchmark
benchmarking::add_to_whitelist(
frame_system::Account::<T>::hashed_key_for(&contract.account_id).into(),
);
// Whitelist the contract's contractInfo as it is already accounted for in the call
// benchmark
benchmarking::add_to_whitelist(
crate::ContractInfoOf::<T>::hashed_key_for(&contract.account_id).into(),
);
With that we are down to the following results for seal_call
/// Storage: `Contracts::ContractInfoOf` (r:1 w:1)
/// Storage: `Contracts::CodeInfoOf` (r:1 w:0)
/// Storage: `Contracts::PristineCode` (r:1 w:0)
/// Storage: `System::Account` (r:1 w:1) (if balance transfer)
/// Storage: `System::EventTopics` (r:2 w:2)
bot bench substrate-pallet --pallet=pallet_contracts
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6228619 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
. 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 3-42013e83-6bb5-4335-910f-7521fb9cd439
to cancel this command or bot cancel
to cancel all commands in this pull request.
@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6228619 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6228619/artifacts/download.
bot bench substrate-pallet --pallet=pallet_contracts
@pgherveou "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
(https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6229744) was cancelled in https://github.com/paritytech/polkadot-sdk/pull/4233#issuecomment-2116914185
bot cancel 1-621accc2-6fcb-4497-a21d-db259c223c49
bot bench substrate-pallet --pallet=pallet_contracts
@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6229744 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6229744/artifacts/download.
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6229891 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
. 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 2-607e1106-171c-4c3f-9d37-c90dbb24718b
to cancel this command or bot cancel
to cancel all commands in this pull request.
@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6229891 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6229891/artifacts/download.
seal_call
still has 3 extra reads and 1 extra write from before.
`seal_call` still has **3** extra reads and **1** extra write from before.
I traced the execution this seems correct, let me check the old equation
Yes you are right. The storage accesses look correct. They were too low before. One last thing. There should be only one access to
Account
forseal_instantiate
to bring it in line withseal_call
.
Yes reason for that is that the bench_call setup is using the contract caller as origin, I am changing it to the root_contract for delegate/call/instantiate, so that the origin account get whitelisted
bot bench substrate-pallet --pallet=pallet_contracts
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6240464 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
. 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 8-1d80a825-c11d-44b3-9f19-a817cffd20cc
to cancel this command or bot cancel
to cancel all commands in this pull request.
@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts
has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6240464 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6240464/artifacts/download.
Something still doesn't add up with the
seal_instantiate
benchmark. The base weight is super low (lower thanseal_call
) and then a lot is added in case of transfer:
mm interesting, will look into this tomorrow. Relaunching one more time to check if this is consistent
bot bench substrate-pallet --pallet=pallet_contracts