Skip to content

Commit

Permalink
Merge pull request #3615 from anoma/grarco/no-parallel-gas
Browse files Browse the repository at this point in the history
Remove parallel gas
  • Loading branch information
mergify[bot] authored Aug 14, 2024
2 parents 125af64 + 0c07a1b commit de6f4cc
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 187 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/improvements/3615-no-parallel-gas.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Removed parallel gas accounting.
([\#3615](https://github.com/anoma/namada/pull/3615))
2 changes: 1 addition & 1 deletion .github/workflows/scripts/hermes.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.10.0-namada-beta15-rc
1.10.0-namada-beta15-rc2
109 changes: 22 additions & 87 deletions crates/gas/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub enum GasParseError {
}

// RAW GAS COSTS
// ================================================================================
// =============================================================================
// The raw gas costs exctracted from the benchmarks.
//
const COMPILE_GAS_PER_BYTE_RAW: u64 = 1_664;
Expand Down Expand Up @@ -119,7 +119,7 @@ const MASP_CONVERT_CHECK_GAS_RAW: u64 = 188_590;
const MASP_OUTPUT_CHECK_GAS_RAW: u64 = 204_430;
// The cost to run the final masp check in the bundle
const MASP_FINAL_CHECK_GAS_RAW: u64 = 43;
// ================================================================================
// =============================================================================

// A correction factor for non-WASM-opcodes costs. We can see that the
// gas cost we get for wasm codes (txs and vps) is much greater than what we
Expand All @@ -134,10 +134,10 @@ const MASP_FINAL_CHECK_GAS_RAW: u64 = 43;
const GAS_COST_CORRECTION: u64 = 5;

// ADJUSTED GAS COSTS
// ================================================================================
// =============================================================================
// The gas costs adjusted for the correction factor.
//
const PARALLEL_GAS_DIVIDER: u64 = 1;

// The compilation cost is reduced by a factor to compensate for the (most
// likely) presence of the cache
const COMPILE_GAS_PER_BYTE: u64 =
Expand Down Expand Up @@ -207,10 +207,7 @@ pub const MASP_OUTPUT_CHECK_GAS: u64 =
/// The cost to run the final masp check in the bundle
pub const MASP_FINAL_CHECK_GAS: u64 =
MASP_FINAL_CHECK_GAS_RAW * GAS_COST_CORRECTION;
/// Gas divider specific for the masp vp. Only allocates half of the cores to
/// the masp vp since we can expect the other half to be busy with other vps
pub const MASP_PARALLEL_GAS_DIVIDER: u64 = 1;
// ================================================================================
// =============================================================================

/// Gas module result for functions that may fail
pub type Result<T> = std::result::Result<T, Error>;
Expand Down Expand Up @@ -375,6 +372,20 @@ pub trait GasMetering {

/// Get the gas limit
fn get_gas_limit(&self) -> Gas;

/// Check if the vps went out of gas. Starts with the gas consumed by the
/// transaction.
fn check_vps_limit(&self, vps_gas: Gas) -> Result<()> {
let total = self
.get_tx_consumed_gas()
.checked_add(vps_gas)
.ok_or(Error::GasOverflow)?;
if total > self.get_gas_limit() {
return Err(Error::TransactionGasExceededError);
}

Ok(())
}
}

/// Gas metering in a transaction
Expand All @@ -400,23 +411,6 @@ pub struct VpGasMeter {
current_gas: Gas,
}

/// Gas meter for VPs parallel runs
#[derive(
Clone,
Debug,
Default,
BorshSerialize,
BorshDeserialize,
BorshDeserializer,
BorshSchema,
Serialize,
Deserialize,
)]
pub struct VpsGas {
max: Gas,
rest: Vec<Gas>,
}

impl GasMetering for TxGasMeter {
fn consume(&mut self, gas: u64) -> Result<()> {
if self.gas_overflow {
Expand Down Expand Up @@ -484,11 +478,6 @@ impl TxGasMeter {
)
}

/// Add the gas cost used in validity predicates to the current transaction.
pub fn add_vps_gas(&mut self, vps_gas: &VpsGas) -> Result<()> {
self.consume(vps_gas.get_current_gas()?.into())
}

/// Get the amount of gas still available to the transaction
pub fn get_available_gas(&self) -> Gas {
self.tx_gas_limit
Expand Down Expand Up @@ -563,64 +552,10 @@ impl VpGasMeter {
current_gas: Gas::default(),
}
}
}

impl VpsGas {
/// Set the gas cost from a VP run. It consumes the [`VpGasMeter`]
/// instance which shouldn't be accessed passed this point.
pub fn set(&mut self, vp_gas_meter: VpGasMeter) -> Result<()> {
if vp_gas_meter.current_gas > self.max {
self.rest.push(self.max);
self.max = vp_gas_meter.current_gas;
} else {
self.rest.push(vp_gas_meter.current_gas);
}

self.check_limit(&vp_gas_meter)
}

/// Merge validity predicates gas meters from parallelized runs. Consumes
/// the other `VpsGas` instance which shouldn't be used passed this point.
pub fn merge(
&mut self,
mut other: VpsGas,
tx_gas_meter: &TxGasMeter,
) -> Result<()> {
if self.max < other.max {
self.rest.push(self.max);
self.max = other.max;
} else {
self.rest.push(other.max);
}
self.rest.append(&mut other.rest);

self.check_limit(tx_gas_meter)
}

/// Check if the vp went out of gas. Starts from the gas consumed by the
/// transaction.
fn check_limit(&self, gas_meter: &impl GasMetering) -> Result<()> {
let total = gas_meter
.get_tx_consumed_gas()
.checked_add(self.get_current_gas()?)
.ok_or(Error::GasOverflow)?;
if total > gas_meter.get_gas_limit() {
return Err(Error::TransactionGasExceededError);
}
Ok(())
}

/// Get the gas consumed by the parallelized VPs
fn get_current_gas(&self) -> Result<Gas> {
let parallel_gas = self
.rest
.iter()
.try_fold(Gas::default(), |acc, gas| {
acc.checked_add(*gas).ok_or(Error::GasOverflow)
})?
.checked_div(PARALLEL_GAS_DIVIDER)
.expect("Div by non-zero int cannot fail");
self.max.checked_add(parallel_gas).ok_or(Error::GasOverflow)
/// Get the gas consumed by the VP alone
pub fn get_vp_consumed_gas(&self) -> Gas {
self.current_gas
}
}

Expand Down
Loading

0 comments on commit de6f4cc

Please sign in to comment.