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.

Sign up for GitHub

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

improve weight per gas calculation #995

Merged
merged 1 commit into from
Feb 8, 2023
Merged

improve weight per gas calculation #995

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 35 additions & 7 deletions template/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use frame_support::weights::constants::RocksDbWeight as RuntimeDbWeight;
use frame_support::{
construct_runtime, parameter_types,
traits::{ConstU32, ConstU8, FindAuthor, KeyOwnerProofSystem, OnTimestampSet},
weights::{constants::WEIGHT_REF_TIME_PER_SECOND, ConstantMultiplier, IdentityFee, Weight},
weights::{constants::WEIGHT_REF_TIME_PER_MILLIS, ConstantMultiplier, IdentityFee, Weight},
};
use pallet_grandpa::{
fg_primitives, AuthorityId as GrandpaId, AuthorityList as GrandpaAuthorityList,
Expand Down Expand Up @@ -136,9 +136,12 @@ pub fn native_version() -> sp_version::NativeVersion {
}

const NORMAL_DISPATCH_RATIO: Perbill = Perbill::from_percent(75);
/// We allow for 2 seconds of compute with a 6 second average block time.
pub const MAXIMUM_BLOCK_WEIGHT: Weight =
Weight::from_parts(2u64 * WEIGHT_REF_TIME_PER_SECOND, u64::MAX);
/// We allow for 2000ms of compute with a 6 second average block time.
pub const WEIGHT_MILLISECS_PER_BLOCK: u64 = 2000;
pub const MAXIMUM_BLOCK_WEIGHT: Weight = Weight::from_parts(
WEIGHT_MILLISECS_PER_BLOCK * WEIGHT_REF_TIME_PER_MILLIS,
u64::MAX,
);
pub const MAXIMUM_BLOCK_LENGTH: u32 = 5 * 1024 * 1024;

parameter_types! {
Expand Down Expand Up @@ -312,11 +315,36 @@ impl<F: FindAuthor<u32>> FindAuthor<H160> for FindAuthorTruncated<F> {
}
}

const WEIGHT_PER_GAS: u64 = 20_000;
/// `WeightPerGas` is an approximate ratio of the amount of Weight per Gas.
/// u64 works for approximations because Weight is a very small unit compared to gas.
///
/// `GAS_PER_MILLIS * WEIGHT_MILLIS_PER_BLOCK * TXN_RATIO ~= BLOCK_GAS_LIMIT`
/// `WEIGHT_PER_GAS = WEIGHT_REF_TIME_PER_MILLIS / GAS_PER_MILLIS
/// = WEIGHT_REF_TIME_PER_MILLIS / (BLOCK_GAS_LIMIT / TXN_RATIO / WEIGHT_MILLIS_PER_BLOCK)
/// = TXN_RATIO * (WEIGHT_REF_TIME_PER_MILLIS * WEIGHT_MILLIS_PER_BLOCK) / BLOCK_GAS_LIMIT`
///
/// For example, given the 2000ms Weight, from which 75% only are used for transactions,
/// the total EVM execution gas limit is `GAS_PER_MILLIS * 2000 * 75% = BLOCK_GAS_LIMIT`.
pub fn weight_per_gas(
block_gas_limit: u64,
txn_ratio: Perbill,
weight_millis_per_block: u64,
) -> u64 {
let weight_per_block = WEIGHT_REF_TIME_PER_MILLIS.saturating_mul(weight_millis_per_block);
let weight_per_gas = (txn_ratio * weight_per_block).saturating_div(block_gas_limit);
assert!(
weight_per_gas >= 1,
"WeightPerGas must greater than or equal with 1"
);
weight_per_gas
}

const BLOCK_GAS_LIMIT: u64 = 75_000_000;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you get this BLOCK_GAS_LIMIT? There seems to be a dead loop in your solution. When you calculate the BLOCK_GAS_LIMIT, you also need WeightPerGas.

Copy link
Collaborator Author

@koushiro koushiro Feb 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, you need to know BLOCK_GAS_LIMIT firstly, then calculate the weight per gas.
if not that, how did you get the original WEIGHT_PER_GAS?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remembered that the const value was estimated by the benchmark.
https://github.com/paritytech/frontier/blob/master/frame/evm/src/benchmarking.rs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just reversed order -- previously we get block gas limit from weight per gas. Now we get weight per gas from block gas limit.

Both are equivalent, but I'd honestly like the new approach slightly more. The thing is -- one can get a much more accurate estimate of the overall block gas limit through benchmarking (because it's aggregated), than weight per gas, and thus fixing the former would be better.

Copy link
Collaborator

@boundless-forest boundless-forest Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on #994 , an issue that has some relevance to the current pr.

previously we get block gas limit from weight per gas.

block_gas_limit was previously calculated from block_max_weight and weight_per_gas. block_max_weight is known in runtime and weight_per_gas can be obtained from pallet_evm's benchmark.

block_gas_limit = block_max_weight / weight_per_gas.

one can get a much more accurate estimate of the overall block gas limit through benchmarking (because it's aggregated)

Can you go a bit deeper into your idea? @sorpaas

Benchmark the maximum value of transaction gas that a block of fixed weight size can hold? That doesn't seem to work, the Ethereum::transact() call's weight calculation requires weight_per_gas, and the weigth_per_gas calculation requires block_gas_limit, which is like a loop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just mean they are equivalent. When you know block_gas_limit, you know weight_per_gas, and vice versa. The formula is always fixed:

  • block_gas_limit = block_max_weight / weight_per_gas
  • weight_per_gas = block_max_weight / block_gas_limit

We just need to get any of the value. Previously we get weight_per_gas first and then calculate block_gas_limit. After this PR we get block_gas_limit first and then calculate weight_per_gas.

I don't see any loops -- we use pallet_evm::Config::WeightPerGas for transaction / execution related gas calculation through the WeightGasMapping. pallet_evm::Config::BlockGasLimit is not used for them, and instead is only used for populating the Ethereum block gas limit. However, indeed, the config value BlockGasLimit should probably belong to pallet_ethereum instead of pallet_evm.

Regarding benchmarking, the normal benchmark process for Substrate is different than benchmarking BlockGasLimit/WeightPerGas. Substrate has strict definition of what weight is (10^12 Weight as 1 second of computation on the physical machine used for benchmarking), and the whole Substrate benchmarking process is based on that. However, for BlockGasLimit/WeightPerGas, the process is different -- we have to figure out the maximum allowable value through a trial and error process.

  • Give an initial block gas limit / weight per gas, execute a series of test blocks, and see if it can finishes in target time.
  • If it finishes significantly faster, increase the value, otherwise, decrease the value, and then repeat.

This is why I say the new method (fixing BlockGasLimit instead of WeightPerGas) is better. Technically the old and new method will get the same result (because it's equivalent). However, using BlockGasLimit is aggregated, and more intuitive because that's the actual value that we're comparing against in benchmarking.


parameter_types! {
pub BlockGasLimit: U256 = U256::from(NORMAL_DISPATCH_RATIO * MAXIMUM_BLOCK_WEIGHT.ref_time() / WEIGHT_PER_GAS);
pub BlockGasLimit: U256 = U256::from(BLOCK_GAS_LIMIT);
pub PrecompilesValue: FrontierPrecompiles<Runtime> = FrontierPrecompiles::<_>::new();
pub WeightPerGas: Weight = Weight::from_ref_time(WEIGHT_PER_GAS);
pub WeightPerGas: Weight = Weight::from_ref_time(weight_per_gas(BLOCK_GAS_LIMIT, NORMAL_DISPATCH_RATIO, WEIGHT_MILLISECS_PER_BLOCK));
}

impl pallet_evm::Config for Runtime {
Expand Down