polkadot-sdk icon indicating copy to clipboard operation
polkadot-sdk copied to clipboard

Contracts: Rework host fn benchmarks

Open pgherveou opened this issue 10 months ago • 29 comments

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]);
}

Weights compare

pgherveou avatar Apr 22 '24 08:04 pgherveou

bot bench substrate-pallet --pallet=pallet_contracts

pgherveou avatar Apr 29 '24 04:04 pgherveou

@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.

command-bot[bot] avatar Apr 29 '24 04:04 command-bot[bot]

@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.

command-bot[bot] avatar Apr 29 '24 05:04 command-bot[bot]

bot bench substrate-pallet --pallet=pallet_contracts

pgherveou avatar Apr 29 '24 09:04 pgherveou

@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.

command-bot[bot] avatar Apr 29 '24 09:04 command-bot[bot]

@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.

command-bot[bot] avatar Apr 29 '24 10:04 command-bot[bot]

bot bench substrate-pallet --pallet=pallet_contracts

pgherveou avatar Apr 29 '24 12:04 pgherveou

@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.

command-bot[bot] avatar Apr 29 '24 12:04 command-bot[bot]

@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.

command-bot[bot] avatar Apr 29 '24 12:04 command-bot[bot]

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: Screenshot 2024-05-07 at 12 50 02

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.

pgherveou avatar May 07 '24 11:05 pgherveou

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.

athei avatar May 08 '24 09:05 athei

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 🤔

pgherveou avatar May 08 '24 14:05 pgherveou

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)

pgherveou avatar May 16 '24 19:05 pgherveou

bot bench substrate-pallet --pallet=pallet_contracts

pgherveou avatar May 16 '24 19:05 pgherveou

@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.

command-bot[bot] avatar May 16 '24 19:05 command-bot[bot]

@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.

command-bot[bot] avatar May 16 '24 19:05 command-bot[bot]

bot bench substrate-pallet --pallet=pallet_contracts

pgherveou avatar May 17 '24 07:05 pgherveou

@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

command-bot[bot] avatar May 17 '24 07:05 command-bot[bot]

bot cancel 1-621accc2-6fcb-4497-a21d-db259c223c49

pgherveou avatar May 17 '24 07:05 pgherveou

bot bench substrate-pallet --pallet=pallet_contracts

pgherveou avatar May 17 '24 07:05 pgherveou

@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.

command-bot[bot] avatar May 17 '24 07:05 command-bot[bot]

@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.

command-bot[bot] avatar May 17 '24 07:05 command-bot[bot]

@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.

command-bot[bot] avatar May 17 '24 08:05 command-bot[bot]

Screenshot 2024-05-17 at 12 50 08

seal_call still has 3 extra reads and 1 extra write from before.

athei avatar May 17 '24 10:05 athei

Screenshot 2024-05-17 at 12 50 08 `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

pgherveou avatar May 17 '24 10:05 pgherveou

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 for seal_instantiate to bring it in line with seal_call. Screenshot 2024-05-17 at 14 49 08

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

pgherveou avatar May 17 '24 14:05 pgherveou

bot bench substrate-pallet --pallet=pallet_contracts

pgherveou avatar May 17 '24 14:05 pgherveou

@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.

command-bot[bot] avatar May 17 '24 14:05 command-bot[bot]

@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.

command-bot[bot] avatar May 17 '24 15:05 command-bot[bot]

Something still doesn't add up with the seal_instantiate benchmark. The base weight is super low (lower than seal_call) and then a lot is added in case of transfer: Screenshot 2024-05-18 at 07 31 52

mm interesting, will look into this tomorrow. Relaunching one more time to check if this is consistent

bot bench substrate-pallet --pallet=pallet_contracts

pgherveou avatar May 19 '24 19:05 pgherveou