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.

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

Reuse compute budget processing #1700

Merged
merged 2 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
),
tao-stones marked this conversation as resolved.
Show resolved Hide resolved
(
"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