From 8f3f06cc7f6eac156c18fd147f954af75fa403a7 Mon Sep 17 00:00:00 2001 From: Tao Zhu <82401714+tao-stones@users.noreply.github.com> Date: Thu, 7 Mar 2024 09:23:49 -0600 Subject: [PATCH] Combine builtin and BPF compute cost in cost model (#29) * Combine builtin and BPF execution cost into programs_execution_cost since VM has started to consume CUs uniformly * update tests * apply suggestions from code review --- core/src/banking_stage.rs | 3 +- core/src/banking_stage/consumer.rs | 15 ++-- core/src/banking_stage/qos_service.rs | 61 +++++---------- cost-model/src/cost_model.rs | 103 ++++++++++++++++---------- cost-model/src/cost_tracker.rs | 16 ++-- cost-model/src/transaction_cost.rs | 25 ++----- 6 files changed, 106 insertions(+), 117 deletions(-) diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 652f2569f..603ff55f0 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -285,8 +285,7 @@ pub struct BatchedTransactionCostDetails { pub batched_signature_cost: u64, pub batched_write_lock_cost: u64, pub batched_data_bytes_cost: u64, - pub batched_builtins_execute_cost: u64, - pub batched_bpf_execute_cost: u64, + pub batched_programs_execute_cost: u64, } #[derive(Debug, Default)] diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index f4ac6c604..957e190c8 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -1549,16 +1549,17 @@ mod tests { assert_eq!(retryable_transaction_indexes, vec![1]); let expected_block_cost = if !apply_cost_tracker_during_replay_enabled { - let actual_bpf_execution_cost = match commit_transactions_result.first().unwrap() { - CommitTransactionDetails::Committed { compute_units } => *compute_units, - CommitTransactionDetails::NotCommitted => { - unreachable!() - } - }; + let actual_programs_execution_cost = + match commit_transactions_result.first().unwrap() { + CommitTransactionDetails::Committed { compute_units } => *compute_units, + CommitTransactionDetails::NotCommitted => { + unreachable!() + } + }; let mut cost = CostModel::calculate_cost(&transactions[0], &bank.feature_set); if let TransactionCost::Transaction(ref mut usage_cost) = cost { - usage_cost.bpf_execution_cost = actual_bpf_execution_cost; + usage_cost.programs_execution_cost = actual_programs_execution_cost; } block_cost + cost.sum() diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index 77f05c73a..8c1507ae3 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -236,14 +236,10 @@ impl QosService { batched_transaction_details.costs.batched_data_bytes_cost, Ordering::Relaxed, ); - self.metrics.stats.estimated_builtins_execute_cu.fetch_add( + self.metrics.stats.estimated_programs_execute_cu.fetch_add( batched_transaction_details .costs - .batched_builtins_execute_cost, - Ordering::Relaxed, - ); - self.metrics.stats.estimated_bpf_execute_cu.fetch_add( - batched_transaction_details.costs.batched_bpf_execute_cost, + .batched_programs_execute_cost, Ordering::Relaxed, ); @@ -297,7 +293,7 @@ impl QosService { pub fn accumulate_actual_execute_cu(&self, units: u64) { self.metrics .stats - .actual_bpf_execute_cu + .actual_programs_execute_cu .fetch_add(units, Ordering::Relaxed); } @@ -331,12 +327,8 @@ impl QosService { saturating_add_assign!( batched_transaction_details .costs - .batched_builtins_execute_cost, - cost.builtins_execution_cost() - ); - saturating_add_assign!( - batched_transaction_details.costs.batched_bpf_execute_cost, - cost.bpf_execution_cost() + .batched_programs_execute_cost, + cost.programs_execution_cost() ); } Err(transaction_error) => match transaction_error { @@ -427,14 +419,11 @@ struct QosServiceMetricsStats { /// accumulated estimated instruction data Compute Units to be packed into block estimated_data_bytes_cu: AtomicU64, - /// accumulated estimated builtin programs Compute Units to be packed into block - estimated_builtins_execute_cu: AtomicU64, - - /// accumulated estimated SBF program Compute Units to be packed into block - estimated_bpf_execute_cu: AtomicU64, + /// accumulated estimated program Compute Units to be packed into block + estimated_programs_execute_cu: AtomicU64, /// accumulated actual program Compute Units that have been packed into block - actual_bpf_execute_cu: AtomicU64, + actual_programs_execute_cu: AtomicU64, /// accumulated actual program execute micro-sec that have been packed into block actual_execute_time_us: AtomicU64, @@ -515,24 +504,19 @@ impl QosServiceMetrics { i64 ), ( - "estimated_builtins_execute_cu", + "estimated_programs_execute_cu", self.stats - .estimated_builtins_execute_cu + .estimated_programs_execute_cu .swap(0, Ordering::Relaxed), i64 ), ( - "estimated_bpf_execute_cu", + "actual_programs_execute_cu", self.stats - .estimated_bpf_execute_cu + .actual_programs_execute_cu .swap(0, Ordering::Relaxed), i64 ), - ( - "actual_bpf_execute_cu", - self.stats.actual_bpf_execute_cu.swap(0, Ordering::Relaxed), - i64 - ), ( "actual_execute_time_us", self.stats.actual_execute_time_us.swap(0, Ordering::Relaxed), @@ -735,7 +719,7 @@ mod tests { let committed_status: Vec = qos_cost_results .iter() .map(|tx_cost| CommitTransactionDetails::Committed { - compute_units: tx_cost.as_ref().unwrap().bpf_execution_cost() + compute_units: tx_cost.as_ref().unwrap().programs_execution_cost() + execute_units_adjustment, }) .collect(); @@ -862,7 +846,7 @@ mod tests { CommitTransactionDetails::NotCommitted } else { CommitTransactionDetails::Committed { - compute_units: tx_cost.as_ref().unwrap().bpf_execution_cost() + compute_units: tx_cost.as_ref().unwrap().programs_execution_cost() + execute_units_adjustment, } } @@ -898,8 +882,7 @@ mod tests { let signature_cost = 1; let write_lock_cost = 2; let data_bytes_cost = 3; - let builtins_execution_cost = 4; - let bpf_execution_cost = 10; + let programs_execution_cost = 10; let num_txs = 4; let tx_cost_results: Vec<_> = (0..num_txs) @@ -909,8 +892,7 @@ mod tests { signature_cost, write_lock_cost, data_bytes_cost, - builtins_execution_cost, - bpf_execution_cost, + programs_execution_cost, ..UsageCostDetails::default() })) } else { @@ -922,8 +904,7 @@ mod tests { let expected_signatures = signature_cost * (num_txs / 2); let expected_write_locks = write_lock_cost * (num_txs / 2); let expected_data_bytes = data_bytes_cost * (num_txs / 2); - let expected_builtins_execution_costs = builtins_execution_cost * (num_txs / 2); - let expected_bpf_execution_costs = bpf_execution_cost * (num_txs / 2); + let expected_programs_execution_costs = programs_execution_cost * (num_txs / 2); let batched_transaction_details = QosService::accumulate_batched_transaction_costs(tx_cost_results.iter()); assert_eq!( @@ -939,14 +920,10 @@ mod tests { batched_transaction_details.costs.batched_data_bytes_cost ); assert_eq!( - expected_builtins_execution_costs, + expected_programs_execution_costs, batched_transaction_details .costs - .batched_builtins_execute_cost - ); - assert_eq!( - expected_bpf_execution_costs, - batched_transaction_details.costs.batched_bpf_execute_cost + .batched_programs_execute_cost ); } } diff --git a/cost-model/src/cost_model.rs b/cost-model/src/cost_model.rs index 1e1573542..b81ea2440 100644 --- a/cost-model/src/cost_model.rs +++ b/cost-model/src/cost_model.rs @@ -93,21 +93,25 @@ impl CostModel { transaction: &SanitizedTransaction, feature_set: &FeatureSet, ) { - let mut builtin_costs = 0u64; - let mut bpf_costs = 0u64; + let mut programs_execution_costs = 0u64; let mut loaded_accounts_data_size_cost = 0u64; let mut data_bytes_len_total = 0u64; let mut compute_unit_limit_is_set = false; + let mut has_user_space_instructions = false; for (program_id, instruction) in transaction.message().program_instructions_iter() { - // to keep the same behavior, look for builtin first - if let Some(builtin_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) { - builtin_costs = builtin_costs.saturating_add(*builtin_cost); - } else { - bpf_costs = bpf_costs - .saturating_add(u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT)) - .min(u64::from(MAX_COMPUTE_UNIT_LIMIT)); - } + let ix_execution_cost = + if let Some(builtin_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) { + *builtin_cost + } else { + has_user_space_instructions = true; + u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT) + }; + + programs_execution_costs = programs_execution_costs + .saturating_add(ix_execution_cost) + .min(u64::from(MAX_COMPUTE_UNIT_LIMIT)); + data_bytes_len_total = data_bytes_len_total.saturating_add(instruction.data.len() as u64); @@ -120,8 +124,6 @@ impl CostModel { } } - // calculate bpf cost based on compute budget instructions - // if failed to process compute_budget instructions, the transaction will not be executed // by `bank`, therefore it should be considered as no execution cost by cost model. match process_compute_budget_instructions(transaction.message().program_instructions_iter()) @@ -132,8 +134,8 @@ impl CostModel { // 'compute_unit_limit_is_set' flag, because compute_budget does not distinguish // builtin and bpf instructions when calculating default compute-unit-limit. (see // compute_budget.rs test `test_process_mixed_instructions_without_compute_budget`) - if bpf_costs > 0 && compute_unit_limit_is_set { - bpf_costs = u64::from(compute_budget_limits.compute_unit_limit); + if has_user_space_instructions && compute_unit_limit_is_set { + programs_execution_costs = u64::from(compute_budget_limits.compute_unit_limit); } if feature_set @@ -146,13 +148,11 @@ impl CostModel { } } Err(_) => { - builtin_costs = 0; - bpf_costs = 0; + programs_execution_costs = 0; } } - tx_cost.builtins_execution_cost = builtin_costs; - tx_cost.bpf_execution_cost = bpf_costs; + tx_cost.programs_execution_cost = programs_execution_costs; tx_cost.loaded_accounts_data_size_cost = loaded_accounts_data_size_cost; tx_cost.data_bytes_cost = data_bytes_len_total / INSTRUCTION_DATA_BYTES_COST; } @@ -304,8 +304,7 @@ mod tests { &simple_transaction, &FeatureSet::all_enabled(), ); - assert_eq!(*expected_execution_cost, tx_cost.builtins_execution_cost); - assert_eq!(0, tx_cost.bpf_execution_cost); + assert_eq!(*expected_execution_cost, tx_cost.programs_execution_cost); assert_eq!(3, tx_cost.data_bytes_cost); } @@ -333,8 +332,10 @@ mod tests { &token_transaction, &FeatureSet::all_enabled(), ); - assert_eq!(0, tx_cost.builtins_execution_cost); - assert_eq!(200_000, tx_cost.bpf_execution_cost); + assert_eq!( + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, + tx_cost.programs_execution_cost + ); assert_eq!(0, tx_cost.data_bytes_cost); } @@ -396,13 +397,8 @@ mod tests { &token_transaction, &FeatureSet::all_enabled(), ); - assert_eq!( - *BUILT_IN_INSTRUCTION_COSTS - .get(&compute_budget::id()) - .unwrap(), - tx_cost.builtins_execution_cost - ); - assert_eq!(12_345, tx_cost.bpf_execution_cost); + // If cu-limit is specified, that would the cost for all programs + assert_eq!(12_345, tx_cost.programs_execution_cost); assert_eq!(1, tx_cost.data_bytes_cost); } @@ -446,8 +442,7 @@ mod tests { &token_transaction, &FeatureSet::all_enabled(), ); - assert_eq!(0, tx_cost.builtins_execution_cost); - assert_eq!(0, tx_cost.bpf_execution_cost); + assert_eq!(0, tx_cost.programs_execution_cost); } #[test] @@ -474,8 +469,7 @@ mod tests { let mut tx_cost = UsageCostDetails::default(); CostModel::get_transaction_cost(&mut tx_cost, &tx, &FeatureSet::all_enabled()); - assert_eq!(expected_cost, tx_cost.builtins_execution_cost); - assert_eq!(0, tx_cost.bpf_execution_cost); + assert_eq!(expected_cost, tx_cost.programs_execution_cost); assert_eq!(6, tx_cost.data_bytes_cost); } @@ -506,8 +500,7 @@ mod tests { let expected_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2; let mut tx_cost = UsageCostDetails::default(); CostModel::get_transaction_cost(&mut tx_cost, &tx, &FeatureSet::all_enabled()); - assert_eq!(0, tx_cost.builtins_execution_cost); - assert_eq!(expected_cost, tx_cost.bpf_execution_cost); + assert_eq!(expected_cost, tx_cost.programs_execution_cost); assert_eq!(0, tx_cost.data_bytes_cost); } @@ -567,7 +560,7 @@ mod tests { let tx_cost = CostModel::calculate_cost(&tx, &FeatureSet::all_enabled()); assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); - assert_eq!(*expected_execution_cost, tx_cost.builtins_execution_cost()); + assert_eq!(*expected_execution_cost, tx_cost.programs_execution_cost()); assert_eq!(2, tx_cost.writable_accounts().len()); assert_eq!( expected_loaded_accounts_data_size_cost, @@ -596,7 +589,7 @@ mod tests { let tx_cost = CostModel::calculate_cost(&tx, &feature_set); assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); - assert_eq!(*expected_execution_cost, tx_cost.builtins_execution_cost()); + assert_eq!(*expected_execution_cost, tx_cost.programs_execution_cost()); assert_eq!(2, tx_cost.writable_accounts().len()); assert_eq!( expected_loaded_accounts_data_size_cost, @@ -635,7 +628,7 @@ mod tests { let tx_cost = CostModel::calculate_cost(&tx, &feature_set); assert_eq!(expected_account_cost, tx_cost.write_lock_cost()); - assert_eq!(expected_execution_cost, tx_cost.builtins_execution_cost()); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); assert_eq!(2, tx_cost.writable_accounts().len()); assert_eq!( expected_loaded_accounts_data_size_cost, @@ -666,7 +659,37 @@ mod tests { let mut tx_cost = UsageCostDetails::default(); CostModel::get_transaction_cost(&mut tx_cost, &transaction, &FeatureSet::all_enabled()); - assert_eq!(expected_builtin_cost, tx_cost.builtins_execution_cost); - assert_eq!(expected_bpf_cost as u64, tx_cost.bpf_execution_cost); + assert_eq!( + expected_builtin_cost + expected_bpf_cost as u64, + tx_cost.programs_execution_cost + ); + } + + #[test] + fn test_transaction_cost_with_mix_instruction_with_cu_limit() { + let (mint_keypair, start_hash) = test_setup(); + + let transaction = + SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( + &[ + system_instruction::transfer(&mint_keypair.pubkey(), &Pubkey::new_unique(), 2), + ComputeBudgetInstruction::set_compute_unit_limit(12_345), + ], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + start_hash, + )); + // transaction has one builtin instruction, and one ComputeBudget::compute_unit_limit + let expected_cost = *BUILT_IN_INSTRUCTION_COSTS + .get(&solana_system_program::id()) + .unwrap() + + BUILT_IN_INSTRUCTION_COSTS + .get(&compute_budget::id()) + .unwrap(); + + let mut tx_cost = UsageCostDetails::default(); + CostModel::get_transaction_cost(&mut tx_cost, &transaction, &FeatureSet::all_enabled()); + + assert_eq!(expected_cost, tx_cost.programs_execution_cost); } } diff --git a/cost-model/src/cost_tracker.rs b/cost-model/src/cost_tracker.rs index 9d2b3b624..8fb092c36 100644 --- a/cost-model/src/cost_tracker.rs +++ b/cost-model/src/cost_tracker.rs @@ -105,7 +105,7 @@ impl CostTracker { estimated_tx_cost: &TransactionCost, actual_execution_units: u64, ) { - let estimated_execution_units = estimated_tx_cost.bpf_execution_cost(); + let estimated_execution_units = estimated_tx_cost.programs_execution_cost(); match actual_execution_units.cmp(&estimated_execution_units) { Ordering::Equal => (), Ordering::Greater => { @@ -307,7 +307,7 @@ mod tests { system_transaction::transfer(mint_keypair, &keypair.pubkey(), 2, *start_hash), ); let mut tx_cost = UsageCostDetails::new_with_capacity(1); - tx_cost.bpf_execution_cost = 5; + tx_cost.programs_execution_cost = 5; tx_cost.writable_accounts.push(mint_keypair.pubkey()); (simple_transaction, TransactionCost::Transaction(tx_cost)) @@ -606,7 +606,7 @@ mod tests { { let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct1, acct2, acct3], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); assert!(testee.try_add(&tx_cost).is_ok()); @@ -624,7 +624,7 @@ mod tests { { let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct2], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); assert!(testee.try_add(&tx_cost).is_ok()); @@ -644,7 +644,7 @@ mod tests { { let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct1, acct2], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); assert!(testee.try_add(&tx_cost).is_err()); @@ -668,7 +668,7 @@ mod tests { let mut testee = CostTracker::new(account_max, block_max, block_max); let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct1, acct2, acct3], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); let mut expected_block_cost = tx_cost.sum(); @@ -755,7 +755,7 @@ mod tests { let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![acct1, acct2, acct3], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); @@ -802,7 +802,7 @@ mod tests { let cost = 100u64; let tx_cost = TransactionCost::Transaction(UsageCostDetails { writable_accounts: vec![Pubkey::new_unique()], - bpf_execution_cost: cost, + programs_execution_cost: cost, ..UsageCostDetails::default() }); diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index 76865fff3..c6e68bfe1 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -35,10 +35,10 @@ impl TransactionCost { } } - pub fn bpf_execution_cost(&self) -> u64 { + pub fn programs_execution_cost(&self) -> u64 { match self { - Self::SimpleVote { .. } => 0, - Self::Transaction(usage_cost) => usage_cost.bpf_execution_cost, + Self::SimpleVote { .. } => solana_vote_program::vote_processor::DEFAULT_COMPUTE_UNITS, + Self::Transaction(usage_cost) => usage_cost.programs_execution_cost, } } @@ -85,13 +85,6 @@ impl TransactionCost { } } - pub fn builtins_execution_cost(&self) -> u64 { - match self { - Self::SimpleVote { .. } => solana_vote_program::vote_processor::DEFAULT_COMPUTE_UNITS, - Self::Transaction(usage_cost) => usage_cost.builtins_execution_cost, - } - } - pub fn writable_accounts(&self) -> &[Pubkey] { match self { Self::SimpleVote { writable_accounts } => writable_accounts, @@ -109,8 +102,7 @@ pub struct UsageCostDetails { pub signature_cost: u64, pub write_lock_cost: u64, pub data_bytes_cost: u64, - pub builtins_execution_cost: u64, - pub bpf_execution_cost: u64, + pub programs_execution_cost: u64, pub loaded_accounts_data_size_cost: u64, pub account_data_size: u64, } @@ -122,8 +114,7 @@ impl Default for UsageCostDetails { signature_cost: 0u64, write_lock_cost: 0u64, data_bytes_cost: 0u64, - builtins_execution_cost: 0u64, - bpf_execution_cost: 0u64, + programs_execution_cost: 0u64, loaded_accounts_data_size_cost: 0u64, account_data_size: 0u64, } @@ -140,8 +131,7 @@ impl PartialEq for UsageCostDetails { self.signature_cost == other.signature_cost && self.write_lock_cost == other.write_lock_cost && self.data_bytes_cost == other.data_bytes_cost - && self.builtins_execution_cost == other.builtins_execution_cost - && self.bpf_execution_cost == other.bpf_execution_cost + && self.programs_execution_cost == other.programs_execution_cost && self.loaded_accounts_data_size_cost == other.loaded_accounts_data_size_cost && self.account_data_size == other.account_data_size && to_hash_set(&self.writable_accounts) == to_hash_set(&other.writable_accounts) @@ -168,8 +158,7 @@ impl UsageCostDetails { self.signature_cost .saturating_add(self.write_lock_cost) .saturating_add(self.data_bytes_cost) - .saturating_add(self.builtins_execution_cost) - .saturating_add(self.bpf_execution_cost) + .saturating_add(self.programs_execution_cost) .saturating_add(self.loaded_accounts_data_size_cost) } }