-
Notifications
You must be signed in to change notification settings - Fork 709
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: Rework host fn benchmarks #4233
Conversation
bot bench substrate-pallet --pallet=pallet_contracts |
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6079780 was started for your command Comment |
…=dev --target_dir=substrate --pallet=pallet_contracts
@pgherveou Command |
bot bench substrate-pallet --pallet=pallet_contracts |
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6082811 was started for your command Comment |
…=dev --target_dir=substrate --pallet=pallet_contracts
@pgherveou Command |
…=dev --target_dir=substrate --pallet=pallet_contracts
@pgherveou Command |
The CI pipeline was cancelled due to failure one of the required jobs. |
bot bench substrate-pallet --pallet=pallet_contracts |
@pgherveou https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6247144 was started for your command Comment |
…=dev --target_dir=substrate --pallet=pallet_contracts
@pgherveou Command |
@athei Actually I think it does adds up so if you count these 3 lines on .saturating_add(Weight::from_parts(44_066_869, 0).saturating_mul(t.into()))
.saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(t.into())))
.saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(t.into())))
// 44_000_000 + 25_000_000 + 100_000_000 ~= 169_000_000 You roughly get what we have in .saturating_add(Weight::from_parts(159_867_027, 0).saturating_mul(t.into())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I wasn't accounting for the reads and writes which are added unconditionally for seal_instantiate
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is very reasonable and I am glad that it has been done. The new resulting numbers look reasonable and should be more precise than before. Thanks @pgherveou !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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]>
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]>
fix #4163
This PR does the following:
Update to pallet-contracts-proc-macro:
Update benchmarks:
Weights compare