Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contracts: Make host function benchmarks architecture independent #4163

Closed
athei opened this issue Apr 17, 2024 · 0 comments · Fixed by #4233
Closed

contracts: Make host function benchmarks architecture independent #4163

athei opened this issue Apr 17, 2024 · 0 comments · Fixed by #4233
Assignees

Comments

@athei
Copy link
Member

athei commented Apr 17, 2024

The problem

We don't benchmark the host functions callable by contracts of pallet-contracts in isolation. We measure the execution of a wasm module (with wasmi) which just calls the host function t times. We then do some linear regression to determine how expensive it is to add one call. This means that in addition to the host function call itself, it measures all the wasm instructions to load the parameters for the host function onto the stack as this grows with t, too.

This is not optimal for two reason: It adds noise to the host function benchmarks as we just benchmarking a small thing in a big operation (executing one host function vs. calling a whole contract). Secondly, it makes our benchmarks architecture dependent since we need to generate wasm modules for each's host function benchmark. The upcoming support of another architecture makes this inconvenient.

Proposal

Instead of benchmarking the whole module we can call the host function directly. This will probably entail changing the macro that defines our host functions to (additionally) generate some code so that we can call host functions directly. Instead of only registering them with wasmi as it is right now. I think this will mostly get most of the job done. Except for memory access from within host functions which depends on the code being run within wasmi. One solution would be to make Runtime generic over the memory access so we can mock it from the benchmarks. Something we would need to do for PolkaVM anyways in order to support both byte codes.

This will cut down the architecture dependent benchmarks down to those:

  • Benchmark the time it takes wasmi to execute one instruction (already exists)
  • Benchmark the time it takes wasmi to execute an empty host function (no work on the host, no args, no return value)
  • Benchmark the time it takes to access n bytes of memory in wasmi as we mocked this in our test
@github-project-automation github-project-automation bot moved this to Backlog 🗒 in Smart Contracts Apr 17, 2024
@pgherveou pgherveou self-assigned this Apr 18, 2024
github-merge-queue bot pushed a commit that referenced this issue May 23, 2024
fix #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

```rust
// 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);
}
```

```rust
// 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](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)

---------

Co-authored-by: command-bot <>
Co-authored-by: Alexander Theißen <[email protected]>
@github-project-automation github-project-automation bot moved this from Backlog 🗒 to Done ✅ in Smart Contracts May 23, 2024
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this issue Jun 5, 2024
fix paritytech#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

```rust
// 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);
}
```

```rust
// 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](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)

---------

Co-authored-by: command-bot <>
Co-authored-by: Alexander Theißen <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this issue Aug 2, 2024
fix paritytech#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

```rust
// 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);
}
```

```rust
// 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](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)

---------

Co-authored-by: command-bot <>
Co-authored-by: Alexander Theißen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants