diff --git a/Cargo.lock b/Cargo.lock index 036881942..5ed15d9fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5489,7 +5489,6 @@ dependencies = [ "ed25519-dalek", "flate2", "fnv", - "histogram", "itertools 0.10.1", "lazy_static", "libsecp256k1 0.6.0", diff --git a/core/benches/banking_stage.rs b/core/benches/banking_stage.rs index 28a21ebc4..416bcb06a 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 d5ce67c35..364276ce5 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -27,7 +27,6 @@ use solana_runtime::{ bank_utils, cost_model::CostModel, cost_tracker::CostTracker, - cost_tracker_stats::CostTrackerStats, transaction_batch::TransactionBatch, vote_sender_types::ReplayVoteSender, }; @@ -440,7 +439,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; @@ -460,7 +458,6 @@ impl BankingStage { *next_leader, banking_stage_stats, cost_model, - cost_tracker_stats, ); Self::update_buffered_packets_with_new_unprocessed( original_unprocessed_indexes, @@ -484,7 +481,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( @@ -591,7 +587,6 @@ impl BankingStage { recorder: &TransactionRecorder, data_budget: &DataBudget, cost_model: &Arc>, - cost_tracker_stats: &mut CostTrackerStats, ) -> BufferedPacketsDecision { let bank_start; let ( @@ -633,7 +628,6 @@ impl BankingStage { banking_stage_stats, recorder, cost_model, - cost_tracker_stats, ); } BufferedPacketsDecision::Forward => { @@ -718,7 +712,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 { let my_pubkey = cluster_info.id(); while !buffered_packets.is_empty() { @@ -735,7 +728,6 @@ impl BankingStage { &recorder, data_budget, &cost_model, - &mut cost_tracker_stats, ); if matches!(decision, BufferedPacketsDecision::Hold) || matches!(decision, BufferedPacketsDecision::ForwardAndHold) @@ -771,7 +763,6 @@ impl BankingStage { duplicates, &recorder, &cost_model, - &mut cost_tracker_stats, ) { Ok(()) | Err(RecvTimeoutError::Timeout) => (), Err(RecvTimeoutError::Disconnected) => break, @@ -1110,7 +1101,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![]; @@ -1153,7 +1143,6 @@ impl BankingStage { .read() .unwrap() .calculate_cost(&tx, demote_program_write_locks), - cost_tracker_stats, ) .is_err() { @@ -1230,7 +1219,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) = @@ -1243,7 +1231,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); @@ -1286,7 +1273,6 @@ impl BankingStage { .read() .unwrap() .calculate_cost(tx, bank.demote_program_write_locks()), - cost_tracker_stats, ); } }); @@ -1334,7 +1320,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. @@ -1357,7 +1342,6 @@ impl BankingStage { bank.demote_program_write_locks(), bank.vote_only_bank(), cost_model, - cost_tracker_stats, ); unprocessed_packet_conversion_time.stop(); @@ -1419,7 +1403,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)?; @@ -1478,7 +1461,6 @@ impl BankingStage { gossip_vote_sender, banking_stage_stats, cost_model, - cost_tracker_stats, ); new_tx_count += processed; @@ -1512,7 +1494,6 @@ impl BankingStage { next_leader, banking_stage_stats, cost_model, - cost_tracker_stats, ); Self::push_unprocessed( buffered_packets, @@ -2825,7 +2806,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. @@ -2843,7 +2823,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()) @@ -2910,7 +2889,6 @@ mod tests { &BankingStageStats::default(), &recorder, &Arc::new(RwLock::new(CostModel::default())), - &mut CostTrackerStats::default(), ); // Check everything is correct. All indexes after `interrupted_iteration` @@ -3170,7 +3148,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); @@ -3186,7 +3163,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()); @@ -3211,7 +3187,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); @@ -3227,7 +3202,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); @@ -3252,7 +3226,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); @@ -3268,7 +3241,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 0698a187f..2522d7975 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 4d04c6751..98c9a7a7f 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -14,6 +14,7 @@ use { 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, @@ -323,7 +324,7 @@ impl ReplayStage { gossip_duplicate_confirmed_slots_receiver: GossipDuplicateConfirmedSlotsReceiver, gossip_verified_vote_hash_receiver: GossipVerifiedVoteHashReceiver, cluster_slots_update_sender: ClusterSlotsUpdateSender, - cost_update_sender: Sender, + cost_update_sender: Sender, voting_sender: Sender, ) -> Self { let ReplayStageConfig { @@ -1986,7 +1987,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, duplicate_slots_to_repair: &mut DuplicateSlotsToRepair, ancestor_hashes_replay_update_sender: &AncestorHashesReplayUpdateSender, ) -> bool { @@ -2084,6 +2085,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 @@ -2150,7 +2158,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 5da773833..95c9add8b 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, commitment::BlockCommitmentCache, cost_model::CostModel, @@ -54,7 +53,7 @@ use std::{ net::UdpSocket, sync::{ atomic::AtomicBool, - mpsc::{channel, Receiver, Sender}, + mpsc::{channel, Receiver}, Arc, Mutex, RwLock, }, thread, @@ -295,10 +294,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 880389aa1..d004c202b 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -31,7 +31,6 @@ use solana_runtime::{ bank_forks::BankForks, cost_model::CostModel, cost_tracker::CostTracker, - cost_tracker_stats::CostTrackerStats, hardened_unpack::{open_genesis_config, MAX_GENESIS_ARCHIVE_UNPACKED_SIZE}, snapshot_archive_info::SnapshotArchiveInfoGetter, snapshot_config::SnapshotConfig, @@ -773,7 +772,6 @@ 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 { num_transactions += entry.transactions.len(); @@ -796,15 +794,11 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String> &transaction, true, // demote_program_write_locks ); - 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 (program_id, _instruction) in transaction.message().program_instructions_iter() @@ -815,12 +809,8 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String> } println!( - "Slot: {}, Entries: {}, Transactions: {}, Programs {}, {:?}", - slot, - num_entries, - num_transactions, - num_programs, - cost_tracker.get_stats() + "Slot: {}, Entries: {}, Transactions: {}, Programs {}", + slot, num_entries, num_transactions, num_programs, ); println!(" Programs: {:?}", program_ids); diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index 858d8234a..93c3f0bce 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -1112,12 +1112,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" @@ -3249,7 +3243,6 @@ dependencies = [ "dir-diff", "flate2", "fnv", - "histogram", "itertools 0.10.1", "lazy_static", "log", diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 7bf5e6c66..68557d775 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -21,7 +21,6 @@ crossbeam-channel = "0.5" dir-diff = "0.3.2" flate2 = "1.0.22" fnv = "1.0.7" -histogram = "0.6.9" itertools = "0.10.1" lazy_static = "1.4.0" log = "0.4.14" diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index b86e65597..6618c70f1 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 1b2c7b1ee..34a871fcc 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -3,9 +3,7 @@ //! - would_transaction_fit(&tx_cost), immutable function to test if tx with tx_cost would fit into current block //! - add_transaction_cost(&tx_cost), mutable function to accumulate tx_cost to tracker. //! -use crate::block_cost_limits::*; -use crate::cost_model::TransactionCost; -use crate::cost_tracker_stats::CostTrackerStats; +use crate::{block_cost_limits::*, cost_model::TransactionCost}; use solana_sdk::{clock::Slot, pubkey::Pubkey, transaction::SanitizedTransaction}; use std::collections::HashMap; @@ -24,9 +22,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 +39,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 +55,66 @@ impl CostTracker { &self, _transaction: &SanitizedTransaction, 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: &SanitizedTransaction, 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; + self.add_transaction(&tx_cost.writable_accounts, &tx_cost.sum()); } pub fn try_add( &mut self, _transaction: &SanitizedTransaction, 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 +129,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 +150,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 +208,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 +223,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 +245,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 +267,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 +288,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()); } } @@ -348,7 +315,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 { @@ -356,19 +323,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 { @@ -376,36 +341,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 40d7469c9..000000000 --- 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 6fa37a60f..5c911ab79 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -26,7 +26,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;