Skip to content

Commit

Permalink
Reuse compute budget processing (#1700)
Browse files Browse the repository at this point in the history
* refactor: reuse compute budget limits

* fix tests
  • Loading branch information
jstarry authored Jun 20, 2024
1 parent 98f5d0e commit 0d1ad0d
Show file tree
Hide file tree
Showing 10 changed files with 115 additions and 86 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions accounts-db/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ rand_chacha = { workspace = true }
serde_bytes = { workspace = true }
# See order-crates-for-publishing.py for using this unusual `path = "."`
solana-accounts-db = { path = ".", features = ["dev-context-only-utils"] }
solana-compute-budget = { workspace = true }
solana-logger = { workspace = true }
solana-sdk = { workspace = true, features = ["dev-context-only-utils"] }
solana-svm = { workspace = true, features = ["dev-context-only-utils"] }
Expand Down
5 changes: 5 additions & 0 deletions accounts-db/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,7 @@ mod tests {
use {
super::*,
assert_matches::assert_matches,
solana_compute_budget::compute_budget_processor::ComputeBudgetLimits,
solana_sdk::{
account::{AccountSharedData, WritableAccount},
address_lookup_table::state::LookupTableMeta,
Expand Down Expand Up @@ -1566,6 +1567,7 @@ mod tests {
program_indices: vec![],
fee_details: FeeDetails::default(),
rollback_accounts: RollbackAccounts::default(),
compute_budget_limits: ComputeBudgetLimits::default(),
rent: 0,
rent_debits: RentDebits::default(),
loaded_accounts_data_size: 0,
Expand All @@ -1576,6 +1578,7 @@ mod tests {
program_indices: vec![],
fee_details: FeeDetails::default(),
rollback_accounts: RollbackAccounts::default(),
compute_budget_limits: ComputeBudgetLimits::default(),
rent: 0,
rent_debits: RentDebits::default(),
loaded_accounts_data_size: 0,
Expand Down Expand Up @@ -1829,6 +1832,7 @@ mod tests {
nonce: nonce.clone(),
fee_payer_account: from_account_pre.clone(),
},
compute_budget_limits: ComputeBudgetLimits::default(),
rent: 0,
rent_debits: RentDebits::default(),
loaded_accounts_data_size: 0,
Expand Down Expand Up @@ -1928,6 +1932,7 @@ mod tests {
rollback_accounts: RollbackAccounts::SameNonceAndFeePayer {
nonce: nonce.clone(),
},
compute_budget_limits: ComputeBudgetLimits::default(),
rent: 0,
rent_debits: RentDebits::default(),
loaded_accounts_data_size: 0,
Expand Down
28 changes: 11 additions & 17 deletions compute-budget/src/compute_budget.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
use {
crate::compute_budget_processor::{
self, process_compute_budget_instructions, DEFAULT_HEAP_COST,
},
solana_sdk::{instruction::CompiledInstruction, pubkey::Pubkey, transaction::Result},
};
use crate::compute_budget_processor::{self, ComputeBudgetLimits, DEFAULT_HEAP_COST};

#[cfg(all(RUSTC_WITH_SPECIALIZATION, feature = "frozen-abi"))]
impl ::solana_frozen_abi::abi_example::AbiExample for ComputeBudget {
Expand Down Expand Up @@ -136,6 +131,16 @@ impl Default for ComputeBudget {
}
}

impl From<ComputeBudgetLimits> for ComputeBudget {
fn from(compute_budget_limits: ComputeBudgetLimits) -> Self {
ComputeBudget {
compute_unit_limit: u64::from(compute_budget_limits.compute_unit_limit),
heap_size: compute_budget_limits.updated_heap_bytes,
..ComputeBudget::default()
}
}
}

impl ComputeBudget {
pub fn new(compute_unit_limit: u64) -> Self {
ComputeBudget {
Expand Down Expand Up @@ -186,17 +191,6 @@ impl ComputeBudget {
}
}

pub fn try_from_instructions<'a>(
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
) -> Result<Self> {
let compute_budget_limits = process_compute_budget_instructions(instructions)?;
Ok(ComputeBudget {
compute_unit_limit: u64::from(compute_budget_limits.compute_unit_limit),
heap_size: compute_budget_limits.updated_heap_bytes,
..ComputeBudget::default()
})
}

/// Returns cost of the Poseidon hash function for the given number of
/// inputs is determined by the following quadratic function:
///
Expand Down
2 changes: 1 addition & 1 deletion compute-budget/src/compute_budget_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub const MIN_HEAP_FRAME_BYTES: u32 = HEAP_LENGTH as u32;
/// anyone in Mainnet-beta today. It can be set by set_loaded_accounts_data_size_limit instruction
pub const MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES: u32 = 64 * 1024 * 1024;

#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct ComputeBudgetLimits {
pub updated_heap_bytes: u32,
pub compute_unit_limit: u32,
Expand Down
11 changes: 11 additions & 0 deletions core/src/banking_stage/consume_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ impl ConsumeWorkerMetrics {
invalid_account_for_fee,
invalid_account_index,
invalid_program_for_execution,
invalid_compute_budget,
not_allowed_during_cluster_maintenance,
invalid_writable_account,
invalid_rent_paying_account,
Expand Down Expand Up @@ -371,6 +372,9 @@ impl ConsumeWorkerMetrics {
self.error_metrics
.invalid_program_for_execution
.fetch_add(*invalid_program_for_execution, Ordering::Relaxed);
self.error_metrics
.invalid_compute_budget
.fetch_add(*invalid_compute_budget, Ordering::Relaxed);
self.error_metrics
.not_allowed_during_cluster_maintenance
.fetch_add(*not_allowed_during_cluster_maintenance, Ordering::Relaxed);
Expand Down Expand Up @@ -561,6 +565,7 @@ struct ConsumeWorkerTransactionErrorMetrics {
invalid_account_for_fee: AtomicUsize,
invalid_account_index: AtomicUsize,
invalid_program_for_execution: AtomicUsize,
invalid_compute_budget: AtomicUsize,
not_allowed_during_cluster_maintenance: AtomicUsize,
invalid_writable_account: AtomicUsize,
invalid_rent_paying_account: AtomicUsize,
Expand Down Expand Up @@ -644,6 +649,12 @@ impl ConsumeWorkerTransactionErrorMetrics {
.swap(0, Ordering::Relaxed),
i64
),
(
"invalid_compute_budget",
self.invalid_compute_budget
.swap(0, Ordering::Relaxed),
i64
),
(
"not_allowed_during_cluster_maintenance",
self.not_allowed_during_cluster_maintenance
Expand Down
12 changes: 0 additions & 12 deletions program-runtime/src/timings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,6 @@ eager_macro_rules! { $eager_1
.feature_set_clone_us,
i64
),
(
"execute_accessories_compute_budget_process_transaction_us",
$self
.execute_accessories
.compute_budget_process_transaction_us,
i64
),
(
"execute_accessories_get_executors_us",
$self.execute_accessories.get_executors_us,
Expand Down Expand Up @@ -348,7 +341,6 @@ impl ExecuteProcessInstructionTimings {
#[derive(Default, Debug)]
pub struct ExecuteAccessoryTimings {
pub feature_set_clone_us: u64,
pub compute_budget_process_transaction_us: u64,
pub get_executors_us: u64,
pub process_message_us: u64,
pub update_executors_us: u64,
Expand All @@ -358,10 +350,6 @@ pub struct ExecuteAccessoryTimings {
impl ExecuteAccessoryTimings {
pub fn accumulate(&mut self, other: &ExecuteAccessoryTimings) {
saturating_add_assign!(self.feature_set_clone_us, other.feature_set_clone_us);
saturating_add_assign!(
self.compute_budget_process_transaction_us,
other.compute_budget_process_transaction_us
);
saturating_add_assign!(self.get_executors_us, other.get_executors_us);
saturating_add_assign!(self.process_message_us, other.process_message_us);
saturating_add_assign!(self.update_executors_us, other.update_executors_us);
Expand Down
44 changes: 19 additions & 25 deletions svm/src/account_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use {
transaction_processing_callback::TransactionProcessingCallback,
},
itertools::Itertools,
solana_compute_budget::compute_budget_processor::process_compute_budget_instructions,
solana_compute_budget::compute_budget_processor::{
process_compute_budget_instructions, ComputeBudgetLimits,
},
solana_program_runtime::loaded_programs::{ProgramCacheEntry, ProgramCacheForTxBatch},
solana_sdk::{
account::{Account, AccountSharedData, ReadableAccount, WritableAccount},
Expand Down Expand Up @@ -45,6 +47,7 @@ pub struct CheckedTransactionDetails {
#[cfg_attr(feature = "dev-context-only-utils", derive(Default))]
pub struct ValidatedTransactionDetails {
pub rollback_accounts: RollbackAccounts,
pub compute_budget_limits: ComputeBudgetLimits,
pub fee_details: FeeDetails,
pub fee_payer_account: AccountSharedData,
pub fee_payer_rent_debit: u64,
Expand All @@ -56,6 +59,7 @@ pub struct LoadedTransaction {
pub program_indices: TransactionProgramIndices,
pub fee_details: FeeDetails,
pub rollback_accounts: RollbackAccounts,
pub compute_budget_limits: ComputeBudgetLimits,
pub rent: TransactionRent,
pub rent_debits: RentDebits,
pub loaded_accounts_data_size: usize,
Expand Down Expand Up @@ -358,6 +362,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
program_indices,
fee_details: tx_details.fee_details,
rollback_accounts: tx_details.rollback_accounts,
compute_budget_limits: tx_details.compute_budget_limits,
rent: tx_rent,
rent_debits,
loaded_accounts_data_size: accumulated_accounts_data_size,
Expand Down Expand Up @@ -1140,10 +1145,9 @@ mod tests {
&mock_bank,
sanitized_transaction.message(),
ValidatedTransactionDetails {
rollback_accounts: RollbackAccounts::default(),
fee_details: FeeDetails::default(),
fee_payer_account: fee_payer_account_data.clone(),
fee_payer_rent_debit,
..ValidatedTransactionDetails::default()
},
&mut error_metrics,
None,
Expand All @@ -1164,6 +1168,7 @@ mod tests {
program_indices: vec![],
fee_details: FeeDetails::default(),
rollback_accounts: RollbackAccounts::default(),
compute_budget_limits: ComputeBudgetLimits::default(),
rent: fee_payer_rent_debit,
rent_debits: expected_rent_debits,
loaded_accounts_data_size: 0,
Expand Down Expand Up @@ -1208,10 +1213,8 @@ mod tests {
&mock_bank,
sanitized_transaction.message(),
ValidatedTransactionDetails {
rollback_accounts: RollbackAccounts::default(),
fee_details: FeeDetails::default(),
fee_payer_account: fee_payer_account_data.clone(),
fee_payer_rent_debit: 0,
..ValidatedTransactionDetails::default()
},
&mut error_metrics,
None,
Expand All @@ -1233,6 +1236,7 @@ mod tests {
program_indices: vec![vec![]],
fee_details: FeeDetails::default(),
rollback_accounts: RollbackAccounts::default(),
compute_budget_limits: ComputeBudgetLimits::default(),
rent: 0,
rent_debits: RentDebits::default(),
loaded_accounts_data_size: 0,
Expand Down Expand Up @@ -1416,10 +1420,8 @@ mod tests {
&mock_bank,
sanitized_transaction.message(),
ValidatedTransactionDetails {
rollback_accounts: RollbackAccounts::default(),
fee_details: FeeDetails::default(),
fee_payer_account: fee_payer_account_data.clone(),
fee_payer_rent_debit: 0,
..ValidatedTransactionDetails::default()
},
&mut error_metrics,
None,
Expand All @@ -1440,6 +1442,7 @@ mod tests {
],
fee_details: FeeDetails::default(),
rollback_accounts: RollbackAccounts::default(),
compute_budget_limits: ComputeBudgetLimits::default(),
program_indices: vec![vec![1]],
rent: 0,
rent_debits: RentDebits::default(),
Expand Down Expand Up @@ -1598,10 +1601,8 @@ mod tests {
&mock_bank,
sanitized_transaction.message(),
ValidatedTransactionDetails {
rollback_accounts: RollbackAccounts::default(),
fee_details: FeeDetails::default(),
fee_payer_account: fee_payer_account_data.clone(),
fee_payer_rent_debit: 0,
..ValidatedTransactionDetails::default()
},
&mut error_metrics,
None,
Expand All @@ -1627,6 +1628,7 @@ mod tests {
program_indices: vec![vec![2, 1]],
fee_details: FeeDetails::default(),
rollback_accounts: RollbackAccounts::default(),
compute_budget_limits: ComputeBudgetLimits::default(),
rent: 0,
rent_debits: RentDebits::default(),
loaded_accounts_data_size: 0,
Expand Down Expand Up @@ -1689,10 +1691,8 @@ mod tests {
&mock_bank,
sanitized_transaction.message(),
ValidatedTransactionDetails {
rollback_accounts: RollbackAccounts::default(),
fee_details: FeeDetails::default(),
fee_payer_account: fee_payer_account_data.clone(),
fee_payer_rent_debit: 0,
..ValidatedTransactionDetails::default()
},
&mut error_metrics,
None,
Expand Down Expand Up @@ -1721,6 +1721,7 @@ mod tests {
program_indices: vec![vec![3, 1], vec![3, 1]],
fee_details: FeeDetails::default(),
rollback_accounts: RollbackAccounts::default(),
compute_budget_limits: ComputeBudgetLimits::default(),
rent: 0,
rent_debits: RentDebits::default(),
loaded_accounts_data_size: 0,
Expand Down Expand Up @@ -1841,10 +1842,8 @@ mod tests {
false,
);
let validation_result = Ok(ValidatedTransactionDetails {
rollback_accounts: RollbackAccounts::default(),
fee_details: FeeDetails::default(),
fee_payer_account: fee_payer_account_data,
fee_payer_rent_debit: 0,
..ValidatedTransactionDetails::default()
});

let results = load_accounts(
Expand Down Expand Up @@ -1884,6 +1883,7 @@ mod tests {
program_indices: vec![vec![3, 1], vec![3, 1]],
fee_details: FeeDetails::default(),
rollback_accounts: RollbackAccounts::default(),
compute_budget_limits: ComputeBudgetLimits::default(),
rent: 0,
rent_debits: RentDebits::default(),
loaded_accounts_data_size: 0,
Expand Down Expand Up @@ -1915,13 +1915,7 @@ mod tests {
false,
);

let validation_result = Ok(ValidatedTransactionDetails {
rollback_accounts: RollbackAccounts::default(),
fee_details: FeeDetails::default(),
fee_payer_account: AccountSharedData::default(),
fee_payer_rent_debit: 0,
});

let validation_result = Ok(ValidatedTransactionDetails::default());
let result = load_accounts(
&mock_bank,
&[sanitized_transaction.clone()],
Expand Down
7 changes: 7 additions & 0 deletions svm/src/transaction_error_metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub struct TransactionErrorMetrics {
pub invalid_account_for_fee: usize,
pub invalid_account_index: usize,
pub invalid_program_for_execution: usize,
pub invalid_compute_budget: usize,
pub not_allowed_during_cluster_maintenance: usize,
pub invalid_writable_account: usize,
pub invalid_rent_paying_account: usize,
Expand Down Expand Up @@ -50,6 +51,7 @@ impl TransactionErrorMetrics {
self.invalid_program_for_execution,
other.invalid_program_for_execution
);
saturating_add_assign!(self.invalid_compute_budget, other.invalid_compute_budget);
saturating_add_assign!(
self.not_allowed_during_cluster_maintenance,
other.not_allowed_during_cluster_maintenance
Expand Down Expand Up @@ -127,6 +129,11 @@ impl TransactionErrorMetrics {
self.invalid_program_for_execution as i64,
i64
),
(
"invalid_compute_budget",
self.invalid_compute_budget as i64,
i64
),
(
"not_allowed_during_cluster_maintenance",
self.not_allowed_during_cluster_maintenance as i64,
Expand Down
Loading

0 comments on commit 0d1ad0d

Please sign in to comment.