diff --git a/Cargo.lock b/Cargo.lock index 8bcc706f79..10a303064c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5426,7 +5426,6 @@ dependencies = [ "dir-diff", "flate2", "fnv", - "histogram", "itertools 0.9.0", "lazy_static", "libc", diff --git a/core/benches/banking_stage.rs b/core/benches/banking_stage.rs index 12db3998ff..22e493cd6a 100644 --- a/core/benches/banking_stage.rs +++ b/core/benches/banking_stage.rs @@ -19,7 +19,6 @@ use solana_perf::test_tx::test_tx; use solana_poh::poh_recorder::{create_test_recorder, WorkingBankEntry}; use solana_runtime::bank::Bank; use solana_runtime::cost_model::CostModel; -use solana_runtime::cost_tracker_stats::CostTrackerStats; use solana_sdk::genesis_config::GenesisConfig; use solana_sdk::hash::Hash; use solana_sdk::message::Message; @@ -95,7 +94,6 @@ fn bench_consume_buffered(bencher: &mut Bencher) { &BankingStageStats::default(), &recorder, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); }); diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index f0d2bad681..e866f81196 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -26,7 +26,6 @@ use solana_runtime::{ bank_utils, cost_model::CostModel, cost_tracker::CostTracker, - cost_tracker_stats::CostTrackerStats, hashed_transaction::HashedTransaction, transaction_batch::TransactionBatch, vote_sender_types::ReplayVoteSender, @@ -437,7 +436,6 @@ impl BankingStage { banking_stage_stats: &BankingStageStats, recorder: &TransactionRecorder, cost_model: &Arc>, - cost_tracker_stats: &mut CostTrackerStats, ) { let mut rebuffered_packets_len = 0; let mut new_tx_count = 0; @@ -457,7 +455,6 @@ impl BankingStage { *next_leader, banking_stage_stats, cost_model, - cost_tracker_stats, ); Self::update_buffered_packets_with_new_unprocessed( original_unprocessed_indexes, @@ -477,7 +474,6 @@ impl BankingStage { gossip_vote_sender, banking_stage_stats, cost_model, - cost_tracker_stats, ); if processed < verified_txs_len || !Bank::should_bank_still_be_processing_txs( @@ -582,7 +578,6 @@ impl BankingStage { recorder: &TransactionRecorder, data_budget: &DataBudget, cost_model: &Arc>, - cost_tracker_stats: &mut CostTrackerStats, ) -> BufferedPacketsDecision { let bank_start; let ( @@ -624,7 +619,6 @@ impl BankingStage { banking_stage_stats, recorder, cost_model, - cost_tracker_stats, ); } BufferedPacketsDecision::Forward => { @@ -710,7 +704,6 @@ impl BankingStage { let socket = UdpSocket::bind("0.0.0.0:0").unwrap(); let mut buffered_packets = VecDeque::with_capacity(batch_limit); let banking_stage_stats = BankingStageStats::new(id); - let mut cost_tracker_stats = CostTrackerStats::new(id, 0); loop { while !buffered_packets.is_empty() { let decision = Self::process_buffered_packets( @@ -726,7 +719,6 @@ impl BankingStage { &recorder, data_budget, &cost_model, - &mut cost_tracker_stats, ); if matches!(decision, BufferedPacketsDecision::Hold) || matches!(decision, BufferedPacketsDecision::ForwardAndHold) @@ -762,7 +754,6 @@ impl BankingStage { duplicates, &recorder, &cost_model, - &mut cost_tracker_stats, ) { Ok(()) | Err(RecvTimeoutError::Timeout) => (), Err(RecvTimeoutError::Disconnected) => break, @@ -1105,7 +1096,6 @@ impl BankingStage { demote_program_write_locks: bool, votes_only: bool, cost_model: &Arc>, - cost_tracker_stats: &mut CostTrackerStats, ) -> (Vec>, Vec, Vec) { let mut retryable_transaction_packet_indexes: Vec = vec![]; @@ -1146,7 +1136,6 @@ impl BankingStage { .read() .unwrap() .calculate_cost(&tx, demote_program_write_locks), - cost_tracker_stats, ) .is_err() { @@ -1235,7 +1224,6 @@ impl BankingStage { gossip_vote_sender: &ReplayVoteSender, banking_stage_stats: &BankingStageStats, cost_model: &Arc>, - cost_tracker_stats: &mut CostTrackerStats, ) -> (usize, usize, Vec) { let mut packet_conversion_time = Measure::start("packet_conversion"); let (transactions, transaction_to_packet_indexes, retryable_packet_indexes) = @@ -1248,7 +1236,6 @@ impl BankingStage { bank.demote_program_write_locks(), bank.vote_only_bank(), cost_model, - cost_tracker_stats, ); packet_conversion_time.stop(); inc_new_counter_info!("banking_stage-packet_conversion", 1); @@ -1291,7 +1278,6 @@ impl BankingStage { .read() .unwrap() .calculate_cost(tx.transaction(), bank.demote_program_write_locks()), - cost_tracker_stats, ); } }); @@ -1339,7 +1325,6 @@ impl BankingStage { next_leader: Option, banking_stage_stats: &BankingStageStats, cost_model: &Arc>, - cost_tracker_stats: &mut CostTrackerStats, ) -> Vec { // Check if we are the next leader. If so, let's not filter the packets // as we'll filter it again while processing the packets. @@ -1362,7 +1347,6 @@ impl BankingStage { bank.demote_program_write_locks(), bank.vote_only_bank(), cost_model, - cost_tracker_stats, ); unprocessed_packet_conversion_time.stop(); @@ -1424,7 +1408,6 @@ impl BankingStage { duplicates: &Arc, PacketHasher)>>, recorder: &TransactionRecorder, cost_model: &Arc>, - cost_tracker_stats: &mut CostTrackerStats, ) -> Result<(), RecvTimeoutError> { let mut recv_time = Measure::start("process_packets_recv"); let mms = verified_receiver.recv_timeout(recv_timeout)?; @@ -1477,7 +1460,6 @@ impl BankingStage { gossip_vote_sender, banking_stage_stats, cost_model, - cost_tracker_stats, ); new_tx_count += processed; @@ -1511,7 +1493,6 @@ impl BankingStage { next_leader, banking_stage_stats, cost_model, - cost_tracker_stats, ); Self::push_unprocessed( buffered_packets, @@ -2845,7 +2826,6 @@ mod tests { &BankingStageStats::default(), &recorder, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); assert_eq!(buffered_packets[0].1.len(), num_conflicting_transactions); // When the poh recorder has a bank, should process all non conflicting buffered packets. @@ -2863,7 +2843,6 @@ mod tests { &BankingStageStats::default(), &recorder, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); if num_expected_unprocessed == 0 { assert!(buffered_packets.is_empty()) @@ -2930,7 +2909,6 @@ mod tests { &BankingStageStats::default(), &recorder, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); // Check everything is correct. All indexes after `interrupted_iteration` @@ -3189,7 +3167,6 @@ mod tests { false, votes_only, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); assert_eq!(2, txs.len()); assert_eq!(vec![0, 1], tx_packet_index); @@ -3205,7 +3182,6 @@ mod tests { false, votes_only, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); assert_eq!(0, txs.len()); assert_eq!(0, tx_packet_index.len()); @@ -3230,7 +3206,6 @@ mod tests { false, votes_only, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); assert_eq!(3, txs.len()); assert_eq!(vec![0, 1, 2], tx_packet_index); @@ -3246,7 +3221,6 @@ mod tests { false, votes_only, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); assert_eq!(2, txs.len()); assert_eq!(vec![0, 2], tx_packet_index); @@ -3271,7 +3245,6 @@ mod tests { false, votes_only, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); assert_eq!(3, txs.len()); assert_eq!(vec![0, 1, 2], tx_packet_index); @@ -3287,7 +3260,6 @@ mod tests { false, votes_only, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); assert_eq!(3, txs.len()); assert_eq!(vec![0, 1, 2], tx_packet_index); diff --git a/core/src/cost_update_service.rs b/core/src/cost_update_service.rs index 753072302b..1e239d8aae 100644 --- a/core/src/cost_update_service.rs +++ b/core/src/cost_update_service.rs @@ -5,7 +5,7 @@ use solana_ledger::blockstore::Blockstore; use solana_measure::measure::Measure; -use solana_runtime::{bank::ExecuteTimings, cost_model::CostModel}; +use solana_runtime::{bank::Bank, bank::ExecuteTimings, cost_model::CostModel}; use solana_sdk::timing::timestamp; use std::{ sync::{ @@ -65,7 +65,12 @@ impl CostUpdateServiceTiming { } } -pub type CostUpdateReceiver = Receiver; +pub enum CostUpdate { + FrozenBank { bank: Arc }, + ExecuteTiming { execute_timings: ExecuteTimings }, +} + +pub type CostUpdateReceiver = Receiver; pub struct CostUpdateService { thread_hdl: JoinHandle<()>, @@ -113,8 +118,15 @@ impl CostUpdateService { update_count = 0_u64; let mut update_cost_model_time = Measure::start("update_cost_model_time"); for cost_update in cost_update_receiver.try_iter() { - dirty |= Self::update_cost_model(&cost_model, &cost_update); - update_count += 1; + match cost_update { + CostUpdate::FrozenBank { bank } => { + bank.read_cost_tracker().unwrap().report_stats(bank.slot()); + } + CostUpdate::ExecuteTiming { execute_timings } => { + dirty |= Self::update_cost_model(&cost_model, &execute_timings); + update_count += 1; + } + } } update_cost_model_time.stop(); diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 0c384eccc6..d8be551b22 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -13,6 +13,7 @@ use crate::{ consensus::{ ComputedBankState, Stake, SwitchForkDecision, Tower, VotedStakes, SWITCH_FORK_THRESHOLD, }, + cost_update_service::CostUpdate, fork_choice::{ForkChoice, SelectVoteAndResetForkResult}, heaviest_subtree_fork_choice::HeaviestSubtreeForkChoice, latest_validator_votes_for_frozen_banks::LatestValidatorVotesForFrozenBanks, @@ -314,7 +315,7 @@ impl ReplayStage { gossip_verified_vote_hash_receiver: GossipVerifiedVoteHashReceiver, cluster_slots_update_sender: ClusterSlotsUpdateSender, voting_sender: Sender, - cost_update_sender: Sender, + cost_update_sender: Sender, ) -> Self { let ReplayStageConfig { my_pubkey, @@ -1690,7 +1691,7 @@ impl ReplayStage { unfrozen_gossip_verified_vote_hashes: &mut UnfrozenGossipVerifiedVoteHashes, latest_validator_votes_for_frozen_banks: &mut LatestValidatorVotesForFrozenBanks, cluster_slots_update_sender: &ClusterSlotsUpdateSender, - cost_update_sender: &Sender, + cost_update_sender: &Sender, ) -> bool { let mut did_complete_bank = false; let mut tx_count = 0; @@ -1783,6 +1784,13 @@ impl ReplayStage { transaction_status_sender.send_transaction_status_freeze_message(&bank); } bank.freeze(); + // report cost tracker stats + cost_update_sender + .send(CostUpdate::FrozenBank { bank: bank.clone() }) + .unwrap_or_else(|err| { + warn!("cost_update_sender failed sending bank stats: {:?}", err) + }); + let bank_hash = bank.hash(); assert_ne!(bank_hash, Hash::default()); // Needs to be updated before `check_slot_agrees_with_cluster()` so that @@ -1836,7 +1844,7 @@ impl ReplayStage { // send accumulated excute-timings to cost_update_service if !execute_timings.details.per_program_timings.is_empty() { cost_update_sender - .send(execute_timings) + .send(CostUpdate::ExecuteTiming { execute_timings }) .unwrap_or_else(|err| warn!("cost_update_sender failed: {:?}", err)); } diff --git a/core/src/tvu.rs b/core/src/tvu.rs index f756529089..0ae4796f5f 100644 --- a/core/src/tvu.rs +++ b/core/src/tvu.rs @@ -39,7 +39,6 @@ use solana_runtime::{ AbsRequestHandler, AbsRequestSender, AccountsBackgroundService, SnapshotRequestHandler, }, accounts_db::AccountShrinkThreshold, - bank::ExecuteTimings, bank_forks::{BankForks, SnapshotConfig}, commitment::BlockCommitmentCache, cost_model::CostModel, @@ -55,7 +54,7 @@ use std::{ net::UdpSocket, sync::{ atomic::AtomicBool, - mpsc::{channel, Receiver, Sender}, + mpsc::{channel, Receiver}, Arc, Mutex, RwLock, }, thread, @@ -290,10 +289,7 @@ impl Tvu { bank_forks.clone(), ); - let (cost_update_sender, cost_update_receiver): ( - Sender, - Receiver, - ) = channel(); + let (cost_update_sender, cost_update_receiver) = channel(); let cost_update_service = CostUpdateService::new( exit.clone(), blockstore.clone(), diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index e1841d8ed7..06c5125160 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -29,7 +29,6 @@ use solana_runtime::{ bank_forks::{ArchiveFormat, BankForks, SnapshotConfig}, cost_model::CostModel, cost_tracker::CostTracker, - cost_tracker_stats::CostTrackerStats, hardened_unpack::{open_genesis_config, MAX_GENESIS_ARCHIVE_UNPACKED_SIZE}, snapshot_utils, snapshot_utils::{SnapshotVersion, DEFAULT_MAX_SNAPSHOTS_TO_RETAIN}, @@ -746,22 +745,17 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String> let mut cost_model = CostModel::default(); cost_model.initialize_cost_table(&blockstore.read_program_costs().unwrap()); let mut cost_tracker = CostTracker::default(); - let mut cost_tracker_stats = CostTrackerStats::default(); for entry in &entries { transactions += entry.transactions.len(); for transaction in &entry.transactions { programs += transaction.message().instructions.len(); let tx_cost = cost_model.calculate_cost(transaction, true); - if cost_tracker - .try_add(transaction, &tx_cost, &mut cost_tracker_stats) - .is_err() - { + let result = cost_tracker.try_add(transaction, &tx_cost); + if result.is_err() { println!( - "Slot: {}, CostModel rejected transaction {:?}, stats {:?}!", - slot, - transaction, - cost_tracker.get_stats() + "Slot: {}, CostModel rejected transaction {:?}, reason {:?}", + slot, transaction, result ); } for instruction in &transaction.message().instructions { @@ -773,12 +767,11 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String> } println!( - "Slot: {}, Entries: {}, Transactions: {}, Programs {}, {:?}", + "Slot: {}, Entries: {}, Transactions: {}, Programs {}", slot, entries.len(), transactions, programs, - cost_tracker.get_stats() ); println!(" Programs: {:?}", program_ids); diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index 815ab77912..65d99f714f 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -1176,12 +1176,6 @@ dependencies = [ "pkg-config", ] -[[package]] -name = "histogram" -version = "0.6.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12cb882ccb290b8646e554b157ab0b71e64e8d5bef775cd66b6531e52d302669" - [[package]] name = "hmac" version = "0.8.1" @@ -3428,7 +3422,6 @@ dependencies = [ "dir-diff", "flate2", "fnv", - "histogram", "itertools 0.9.0", "lazy_static", "libc", diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 6c795645f2..008cce0ae8 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -21,7 +21,6 @@ crossbeam-channel = "0.4" dir-diff = "0.3.2" flate2 = "1.0.14" fnv = "1.0.7" -histogram = "0.6.9" itertools = "0.9.0" lazy_static = "1.4.0" libc = "0.2.81" diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index 2843a6a5a2..2c7a27ff23 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -42,36 +42,18 @@ impl TransactionCost { } } -#[derive(AbiExample, Debug)] +#[derive(Debug, Default)] pub struct CostModel { - account_cost_limit: u64, - block_cost_limit: u64, instruction_execution_cost_table: ExecuteCostTable, } -impl Default for CostModel { - fn default() -> Self { - CostModel::new(MAX_WRITABLE_ACCOUNT_UNITS, MAX_BLOCK_UNITS) - } -} - impl CostModel { - pub fn new(account_max: u64, block_max: u64) -> Self { + pub fn new() -> Self { Self { - account_cost_limit: account_max, - block_cost_limit: block_max, instruction_execution_cost_table: ExecuteCostTable::default(), } } - pub fn get_account_cost_limit(&self) -> u64 { - self.account_cost_limit - } - - pub fn get_block_cost_limit(&self) -> u64 { - self.block_cost_limit - } - pub fn initialize_cost_table(&mut self, cost_table: &[(Pubkey, u64)]) { cost_table .iter() diff --git a/runtime/src/cost_tracker.rs b/runtime/src/cost_tracker.rs index 7c9b51f992..ae08a70492 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -5,7 +5,6 @@ //! use crate::block_cost_limits::*; use crate::cost_model::TransactionCost; -use crate::cost_tracker_stats::CostTrackerStats; use solana_sdk::{clock::Slot, pubkey::Pubkey, transaction::Transaction}; use std::collections::HashMap; @@ -24,9 +23,9 @@ pub enum CostTrackerError { pub struct CostTracker { account_cost_limit: u64, block_cost_limit: u64, - current_bank_slot: Slot, cost_by_writable_accounts: HashMap, block_cost: u64, + transaction_count: u64, } impl Default for CostTracker { @@ -41,9 +40,9 @@ impl CostTracker { Self { account_cost_limit, block_cost_limit, - current_bank_slot: 0, cost_by_writable_accounts: HashMap::with_capacity(WRITABLE_ACCOUNTS_PER_BLOCK), block_cost: 0, + transaction_count: 0, } } @@ -57,44 +56,62 @@ impl CostTracker { &self, _transaction: &Transaction, tx_cost: &TransactionCost, - stats: &mut CostTrackerStats, ) -> Result<(), CostTrackerError> { - self.would_fit(&tx_cost.writable_accounts, &tx_cost.sum(), stats) + self.would_fit(&tx_cost.writable_accounts, &tx_cost.sum()) } - pub fn add_transaction_cost( - &mut self, - _transaction: &Transaction, - tx_cost: &TransactionCost, - stats: &mut CostTrackerStats, - ) { - let cost = tx_cost.sum(); - self.add_transaction(&tx_cost.writable_accounts, &cost); - - stats.transaction_count += 1; - stats.block_cost += cost; + pub fn add_transaction_cost(&mut self, _transaction: &Transaction, tx_cost: &TransactionCost) { + self.add_transaction(&tx_cost.writable_accounts, &tx_cost.sum()); } pub fn try_add( &mut self, _transaction: &Transaction, tx_cost: &TransactionCost, - stats: &mut CostTrackerStats, ) -> Result { let cost = tx_cost.sum(); - self.would_fit(&tx_cost.writable_accounts, &cost, stats)?; + self.would_fit(&tx_cost.writable_accounts, &cost)?; self.add_transaction(&tx_cost.writable_accounts, &cost); Ok(self.block_cost) } - fn would_fit( - &self, - keys: &[Pubkey], - cost: &u64, - stats: &mut CostTrackerStats, - ) -> Result<(), CostTrackerError> { - stats.transaction_cost_histogram.increment(*cost).unwrap(); + pub fn report_stats(&self, bank_slot: Slot) { + // skip reporting if block is empty + if self.transaction_count == 0 { + return; + } + let (costliest_account, costliest_account_cost) = self.find_costliest_account(); + + datapoint_info!( + "cost_tracker_stats", + ("bank_slot", bank_slot as i64, i64), + ("block_cost", self.block_cost as i64, i64), + ("transaction_count", self.transaction_count as i64, i64), + ( + "number_of_accounts", + self.cost_by_writable_accounts.len() as i64, + i64 + ), + ("costliest_account", costliest_account.to_string(), String), + ("costliest_account_cost", costliest_account_cost as i64, i64), + ); + } + + fn find_costliest_account(&self) -> (Pubkey, u64) { + let mut costliest_account = Pubkey::default(); + let mut costliest_account_cost = 0; + for (key, cost) in self.cost_by_writable_accounts.iter() { + if *cost > costliest_account_cost { + costliest_account = *key; + costliest_account_cost = *cost; + } + } + + (costliest_account, costliest_account_cost) + } + + fn would_fit(&self, keys: &[Pubkey], cost: &u64) -> Result<(), CostTrackerError> { // check against the total package cost if self.block_cost + cost > self.block_cost_limit { return Err(CostTrackerError::WouldExceedBlockMaxLimit); @@ -109,11 +126,6 @@ impl CostTracker { for account_key in keys.iter() { match self.cost_by_writable_accounts.get(&account_key) { Some(chained_cost) => { - stats - .writable_accounts_cost_histogram - .increment(*chained_cost) - .unwrap(); - if chained_cost + cost > self.account_cost_limit { return Err(CostTrackerError::WouldExceedAccountMaxLimit); } else { @@ -135,37 +147,7 @@ impl CostTracker { .or_insert(0) += cost; } self.block_cost += cost; - } -} - -// CostStats can be collected by util, such as ledger_tool -#[derive(Default, Debug)] -pub struct CostStats { - pub bank_slot: Slot, - pub total_cost: u64, - pub number_of_accounts: usize, - pub costliest_account: Pubkey, - pub costliest_account_cost: u64, -} - -impl CostTracker { - pub fn get_stats(&self) -> CostStats { - let mut stats = CostStats { - bank_slot: self.current_bank_slot, - total_cost: self.block_cost, - number_of_accounts: self.cost_by_writable_accounts.len(), - costliest_account: Pubkey::default(), - costliest_account_cost: 0, - }; - - for (key, cost) in self.cost_by_writable_accounts.iter() { - if cost > &stats.costliest_account_cost { - stats.costliest_account = *key; - stats.costliest_account_cost = *cost; - } - } - - stats + self.transaction_count += 1; } } @@ -223,9 +205,7 @@ mod tests { // build testee to have capacity for one simple transaction let mut testee = CostTracker::new(cost, cost); - assert!(testee - .would_fit(&keys, &cost, &mut CostTrackerStats::default()) - .is_ok()); + assert!(testee.would_fit(&keys, &cost).is_ok()); testee.add_transaction(&keys, &cost); assert_eq!(cost, testee.block_cost); } @@ -240,15 +220,11 @@ mod tests { // build testee to have capacity for two simple transactions, with same accounts let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2); { - assert!(testee - .would_fit(&keys1, &cost1, &mut CostTrackerStats::default()) - .is_ok()); + assert!(testee.would_fit(&keys1, &cost1).is_ok()); testee.add_transaction(&keys1, &cost1); } { - assert!(testee - .would_fit(&keys2, &cost2, &mut CostTrackerStats::default()) - .is_ok()); + assert!(testee.would_fit(&keys2, &cost2).is_ok()); testee.add_transaction(&keys2, &cost2); } assert_eq!(cost1 + cost2, testee.block_cost); @@ -266,15 +242,11 @@ mod tests { // build testee to have capacity for two simple transactions, with same accounts let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2); { - assert!(testee - .would_fit(&keys1, &cost1, &mut CostTrackerStats::default()) - .is_ok()); + assert!(testee.would_fit(&keys1, &cost1).is_ok()); testee.add_transaction(&keys1, &cost1); } { - assert!(testee - .would_fit(&keys2, &cost2, &mut CostTrackerStats::default()) - .is_ok()); + assert!(testee.would_fit(&keys2, &cost2).is_ok()); testee.add_transaction(&keys2, &cost2); } assert_eq!(cost1 + cost2, testee.block_cost); @@ -292,16 +264,12 @@ mod tests { let mut testee = CostTracker::new(cmp::min(cost1, cost2), cost1 + cost2); // should have room for first transaction { - assert!(testee - .would_fit(&keys1, &cost1, &mut CostTrackerStats::default()) - .is_ok()); + assert!(testee.would_fit(&keys1, &cost1).is_ok()); testee.add_transaction(&keys1, &cost1); } // but no more sapce on the same chain (same signer account) { - assert!(testee - .would_fit(&keys2, &cost2, &mut CostTrackerStats::default()) - .is_err()); + assert!(testee.would_fit(&keys2, &cost2).is_err()); } } @@ -317,16 +285,12 @@ mod tests { let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1); // should have room for first transaction { - assert!(testee - .would_fit(&keys1, &cost1, &mut CostTrackerStats::default()) - .is_ok()); + assert!(testee.would_fit(&keys1, &cost1).is_ok()); testee.add_transaction(&keys1, &cost1); } // but no more room for package as whole { - assert!(testee - .would_fit(&keys2, &cost2, &mut CostTrackerStats::default()) - .is_err()); + assert!(testee.would_fit(&keys2, &cost2).is_err()); } } @@ -347,7 +311,7 @@ mod tests { // case 1: a tx writes to 3 accounts, should success, we will have: // | acct1 | $cost | // | acct2 | $cost | - // | acct2 | $cost | + // | acct3 | $cost | // and block_cost = $cost { let tx_cost = TransactionCost { @@ -355,19 +319,17 @@ mod tests { execution_cost: cost, ..TransactionCost::default() }; - assert!(testee - .try_add(&tx, &tx_cost, &mut CostTrackerStats::default()) - .is_ok()); - let stat = testee.get_stats(); - assert_eq!(cost, stat.total_cost); - assert_eq!(3, stat.number_of_accounts); - assert_eq!(cost, stat.costliest_account_cost); + assert!(testee.try_add(&tx, &tx_cost).is_ok()); + let (_costliest_account, costliest_account_cost) = testee.find_costliest_account(); + assert_eq!(cost, testee.block_cost); + assert_eq!(3, testee.cost_by_writable_accounts.len()); + assert_eq!(cost, costliest_account_cost); } // case 2: add tx writes to acct2 with $cost, should succeed, result to // | acct1 | $cost | // | acct2 | $cost * 2 | - // | acct2 | $cost | + // | acct3 | $cost | // and block_cost = $cost * 2 { let tx_cost = TransactionCost { @@ -375,36 +337,32 @@ mod tests { execution_cost: cost, ..TransactionCost::default() }; - assert!(testee - .try_add(&tx, &tx_cost, &mut CostTrackerStats::default()) - .is_ok()); - let stat = testee.get_stats(); - assert_eq!(cost * 2, stat.total_cost); - assert_eq!(3, stat.number_of_accounts); - assert_eq!(cost * 2, stat.costliest_account_cost); - assert_eq!(acct2, stat.costliest_account); + assert!(testee.try_add(&tx, &tx_cost).is_ok()); + let (costliest_account, costliest_account_cost) = testee.find_costliest_account(); + assert_eq!(cost * 2, testee.block_cost); + assert_eq!(3, testee.cost_by_writable_accounts.len()); + assert_eq!(cost * 2, costliest_account_cost); + assert_eq!(acct2, costliest_account); } // case 3: add tx writes to [acct1, acct2], acct2 exceeds limit, should failed atomically, // we shoudl still have: // | acct1 | $cost | - // | acct2 | $cost | - // | acct2 | $cost | - // and block_cost = $cost + // | acct2 | $cost * 2 | + // | acct3 | $cost | + // and block_cost = $cost * 2 { let tx_cost = TransactionCost { writable_accounts: vec![acct1, acct2], execution_cost: cost, ..TransactionCost::default() }; - assert!(testee - .try_add(&tx, &tx_cost, &mut CostTrackerStats::default()) - .is_err()); - let stat = testee.get_stats(); - assert_eq!(cost * 2, stat.total_cost); - assert_eq!(3, stat.number_of_accounts); - assert_eq!(cost * 2, stat.costliest_account_cost); - assert_eq!(acct2, stat.costliest_account); + assert!(testee.try_add(&tx, &tx_cost).is_err()); + let (costliest_account, costliest_account_cost) = testee.find_costliest_account(); + assert_eq!(cost * 2, testee.block_cost); + assert_eq!(3, testee.cost_by_writable_accounts.len()); + assert_eq!(cost * 2, costliest_account_cost); + assert_eq!(acct2, costliest_account); } } } diff --git a/runtime/src/cost_tracker_stats.rs b/runtime/src/cost_tracker_stats.rs deleted file mode 100644 index 40d7469c98..0000000000 --- a/runtime/src/cost_tracker_stats.rs +++ /dev/null @@ -1,75 +0,0 @@ -//! The Stats is not thread safe, each thread should have its own -//! instance of stat with `id`; Stat reports and reset for each slot. -#[derive(Debug, Default)] -pub struct CostTrackerStats { - pub id: u32, - pub transaction_cost_histogram: histogram::Histogram, - pub writable_accounts_cost_histogram: histogram::Histogram, - pub transaction_count: u64, - pub block_cost: u64, - pub bank_slot: u64, -} - -impl CostTrackerStats { - pub fn new(id: u32, bank_slot: u64) -> Self { - CostTrackerStats { - id, - bank_slot, - ..CostTrackerStats::default() - } - } - - pub fn report(&self) { - datapoint_info!( - "cost_tracker_stats", - ("id", self.id as i64, i64), - ( - "transaction_cost_unit_min", - self.transaction_cost_histogram.minimum().unwrap_or(0), - i64 - ), - ( - "transaction_cost_unit_max", - self.transaction_cost_histogram.maximum().unwrap_or(0), - i64 - ), - ( - "transaction_cost_unit_mean", - self.transaction_cost_histogram.mean().unwrap_or(0), - i64 - ), - ( - "transaction_cost_unit_2nd_std", - self.transaction_cost_histogram - .percentile(95.0) - .unwrap_or(0), - i64 - ), - ( - "writable_accounts_cost_min", - self.writable_accounts_cost_histogram.minimum().unwrap_or(0), - i64 - ), - ( - "writable_accounts_cost_max", - self.writable_accounts_cost_histogram.maximum().unwrap_or(0), - i64 - ), - ( - "writable_accounts_cost_mean", - self.writable_accounts_cost_histogram.mean().unwrap_or(0), - i64 - ), - ( - "writable_accounts_cost_2nd_std", - self.writable_accounts_cost_histogram - .percentile(95.0) - .unwrap_or(0), - i64 - ), - ("transaction_count", self.transaction_count as i64, i64), - ("block_cost", self.block_cost as i64, i64), - ("bank_slot", self.bank_slot as i64, i64), - ); - } -} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index ffa1c45087..810572d179 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -21,7 +21,6 @@ pub mod commitment; pub mod contains; pub mod cost_model; pub mod cost_tracker; -pub mod cost_tracker_stats; pub mod epoch_stakes; pub mod execute_cost_table; pub mod genesis_utils;