Skip to content

Commit

Permalink
Measure per byte and not kb for certain benchmarks (paritytech#10863)
Browse files Browse the repository at this point in the history
  • Loading branch information
athei authored and grishasobol committed Mar 28, 2022
1 parent b89cdcb commit 4065713
Show file tree
Hide file tree
Showing 5 changed files with 645 additions and 648 deletions.
28 changes: 14 additions & 14 deletions frame/contracts/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ benchmarks! {
// first time after a new schedule was deployed: For every new schedule a contract needs
// to re-run the instrumentation once.
reinstrument {
let c in 0 .. T::Schedule::get().limits.code_len / 1024;
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c * 1024, Location::Call);
let c in 0 .. T::Schedule::get().limits.code_len;
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c, Location::Call);
Contracts::<T>::store_code_raw(code, whitelisted_caller())?;
let schedule = T::Schedule::get();
let mut gas_meter = GasMeter::new(Weight::MAX);
Expand All @@ -241,15 +241,15 @@ benchmarks! {
Contracts::<T>::reinstrument_module(&mut module, &schedule)?;
}

// This benchmarks the overhead of loading a code of size `c` kb from storage and into
// This benchmarks the overhead of loading a code of size `c` byte from storage and into
// the sandbox. This does **not** include the actual execution for which the gas meter
// is responsible. This is achieved by generating all code to the `deploy` function
// which is in the wasm module but not executed on `call`.
// The results are supposed to be used as `call_with_code_kb(c) - call_with_code_kb(0)`.
call_with_code_kb {
let c in 0 .. T::Schedule::get().limits.code_len / 1024;
call_with_code_per_byte {
let c in 0 .. T::Schedule::get().limits.code_len;
let instance = Contract::<T>::with_caller(
whitelisted_caller(), WasmModule::sized(c * 1024, Location::Deploy), vec![],
whitelisted_caller(), WasmModule::sized(c, Location::Deploy), vec![],
)?;
let value = T::Currency::minimum_balance();
let origin = RawOrigin::Signed(instance.caller.clone());
Expand All @@ -271,13 +271,13 @@ benchmarks! {
// We cannot let `c` grow to the maximum code size because the code is not allowed
// to be larger than the maximum size **after instrumentation**.
instantiate_with_code {
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::Schedule::get().limits.code_len) / 1024;
let s in 0 .. code::max_pages::<T>() * 64;
let salt = vec![42u8; (s * 1024) as usize];
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::Schedule::get().limits.code_len);
let s in 0 .. code::max_pages::<T>() * 64 * 1024;
let salt = vec![42u8; s as usize];
let value = T::Currency::minimum_balance();
let caller = whitelisted_caller();
T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c * 1024, Location::Call);
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c, Location::Call);
let origin = RawOrigin::Signed(caller.clone());
let addr = Contracts::<T>::contract_address(&caller, &hash, &salt);
}: _(origin, value, Weight::MAX, None, code, vec![], salt)
Expand All @@ -299,8 +299,8 @@ benchmarks! {
// Instantiate uses a dummy contract constructor to measure the overhead of the instantiate.
// `s`: Size of the salt in kilobytes.
instantiate {
let s in 0 .. code::max_pages::<T>() * 64;
let salt = vec![42u8; (s * 1024) as usize];
let s in 0 .. code::max_pages::<T>() * 64 * 1024;
let salt = vec![42u8; s as usize];
let value = T::Currency::minimum_balance();
let caller = whitelisted_caller();
T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
Expand Down Expand Up @@ -360,10 +360,10 @@ benchmarks! {
// We cannot let `c` grow to the maximum code size because the code is not allowed
// to be larger than the maximum size **after instrumentation**.
upload_code {
let c in 0 .. Perbill::from_percent(50).mul_ceil(T::Schedule::get().limits.code_len / 1024);
let c in 0 .. Perbill::from_percent(50).mul_ceil(T::Schedule::get().limits.code_len);
let caller = whitelisted_caller();
T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c * 1024, Location::Call);
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c, Location::Call);
let origin = RawOrigin::Signed(caller.clone());
}: _(origin, code, None)
verify {
Expand Down
13 changes: 5 additions & 8 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,7 @@ pub mod pallet {
/// - The `value` is transferred to the new account.
/// - The `deploy` function is executed in the context of the newly-created account.
#[pallet::weight(
T::WeightInfo::instantiate_with_code(
code.len() as u32 / 1024,
salt.len() as u32 / 1024,
)
T::WeightInfo::instantiate_with_code(code.len() as u32, salt.len() as u32)
.saturating_add(*gas_limit)
)]
pub fn instantiate_with_code(
Expand Down Expand Up @@ -445,7 +442,7 @@ pub mod pallet {
}
output.gas_meter.into_dispatch_result(
output.result.map(|(_address, result)| result),
T::WeightInfo::instantiate_with_code(code_len / 1024, salt_len / 1024),
T::WeightInfo::instantiate_with_code(code_len, salt_len),
)
}

Expand All @@ -455,7 +452,7 @@ pub mod pallet {
/// code deployment step. Instead, the `code_hash` of an on-chain deployed wasm binary
/// must be supplied.
#[pallet::weight(
T::WeightInfo::instantiate(salt.len() as u32 / 1024).saturating_add(*gas_limit)
T::WeightInfo::instantiate(salt.len() as u32).saturating_add(*gas_limit)
)]
pub fn instantiate(
origin: OriginFor<T>,
Expand Down Expand Up @@ -485,7 +482,7 @@ pub mod pallet {
}
output.gas_meter.into_dispatch_result(
output.result.map(|(_address, output)| output),
T::WeightInfo::instantiate(salt_len / 1024),
T::WeightInfo::instantiate(salt_len),
)
}

Expand All @@ -505,7 +502,7 @@ pub mod pallet {
/// To avoid this situation a constructor could employ access control so that it can
/// only be instantiated by permissioned entities. The same is true when uploading
/// through [`Self::instantiate_with_code`].
#[pallet::weight(T::WeightInfo::upload_code(code.len() as u32 / 1024))]
#[pallet::weight(T::WeightInfo::upload_code(code.len() as u32))]
pub fn upload_code(
origin: OriginFor<T>,
code: Vec<u8>,
Expand Down
2 changes: 1 addition & 1 deletion frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1998,7 +1998,7 @@ fn reinstrument_does_charge() {
assert!(result2.gas_consumed > result1.gas_consumed);
assert_eq!(
result2.gas_consumed,
result1.gas_consumed + <Test as Config>::WeightInfo::reinstrument(code_len / 1024),
result1.gas_consumed + <Test as Config>::WeightInfo::reinstrument(code_len),
);
});
}
Expand Down
6 changes: 3 additions & 3 deletions frame/contracts/src/wasm/code_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,9 @@ impl<T: Config> Token<T> for CodeToken {
// point because when charging the general weight for calling the contract we not know the
// size of the contract.
match *self {
Reinstrument(len) => T::WeightInfo::reinstrument(len / 1024),
Load(len) => T::WeightInfo::call_with_code_kb(len / 1024)
.saturating_sub(T::WeightInfo::call_with_code_kb(0)),
Reinstrument(len) => T::WeightInfo::reinstrument(len),
Load(len) => T::WeightInfo::call_with_code_per_byte(len)
.saturating_sub(T::WeightInfo::call_with_code_per_byte(0)),
}
}
}
Loading

0 comments on commit 4065713

Please sign in to comment.