Skip to content

Commit

Permalink
wip - remove cost_tracker from tpu; cost_model return tx_cost to clea…
Browse files Browse the repository at this point in the history 
…n up calculate_cost read op api;
  • Loading branch information
@tao-stones
tao-stones committed Oct 12, 2021
1 parent f97212c commit b23420b
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 121 deletions.
6 changes: 2 additions & 4 deletions banking-bench/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use solana_perf::packet::to_packets_chunked;
use solana_poh::poh_recorder::{create_test_recorder, PohRecorder, WorkingBankEntry};
use solana_runtime::{
accounts_background_service::AbsRequestSender, bank::Bank, bank_forks::BankForks,
cost_model::CostModel, cost_tracker::CostTracker,
cost_model::CostModel,
};
use solana_sdk::{
hash::Hash,
Expand Down Expand Up @@ -233,9 +233,7 @@ fn main() {
vote_receiver,
None,
replay_vote_sender,
Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
CostModel::default(),
))))),
Arc::new(RwLock::new(CostModel::default())),
);
poh_recorder.lock().unwrap().set_bank(&bank);

Expand Down
9 changes: 4 additions & 5 deletions core/benches/banking_stage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::CostTracker;
use solana_runtime::cost_tracker_stats::CostTrackerStats;
use solana_sdk::genesis_config::GenesisConfig;
use solana_sdk::hash::Hash;
Expand Down Expand Up @@ -95,9 +94,9 @@ fn bench_consume_buffered(bencher: &mut Bencher) {
None::<Box<dyn Fn()>>,
&BankingStageStats::default(),
&recorder,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
&Arc::new(RwLock::new(
CostModel::new(std::u64::MAX, std::u64::MAX),
))))),
)),
&mut CostTrackerStats::default(),
);
});
Expand Down Expand Up @@ -225,9 +224,9 @@ fn bench_banking(bencher: &mut Bencher, tx_type: TransactionType) {
vote_receiver,
None,
s,
Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
Arc::new(RwLock::new(
CostModel::new(std::u64::MAX, std::u64::MAX),
))))),
)),
);
poh_recorder.lock().unwrap().set_bank(&bank);

Expand Down
54 changes: 28 additions & 26 deletions core/src/banking_stage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ use solana_perf::{
use solana_poh::poh_recorder::{BankStart, PohRecorder, PohRecorderError, TransactionRecorder};
use solana_runtime::{
accounts_db::ErrorCounters,
cost_model::CostModel,
bank::{
Bank, ExecuteTimings, TransactionBalancesSet, TransactionCheckResult,
TransactionExecutionResult,
},
bank_utils,
cost_model::CostModel,
cost_tracker::CostTracker,
cost_tracker_stats::CostTrackerStats,
transaction_batch::TransactionBatch,
Expand Down Expand Up @@ -963,10 +963,9 @@ impl BankingStage {
transaction_status_sender: Option<TransactionStatusSender>,
gossip_vote_sender: &ReplayVoteSender,
) -> (Result<usize, PohRecorderError>, Vec<usize>) {

// TODO TAO - before locking account for txs, do
// 1. calculate TXs costs: cost_model.calculate_costs(txs) -> [transaction_cost]
// 2, combine txs and [transaction_costs] into txs_costs, do packing logic:
// TODO TAO - before locking account for txs, do
// 1. calculate TXs costs: cost_model.calculate_costs(txs) -> [transaction_cost]
// 2, combine txs and [transaction_costs] into txs_costs, do packing logic:
// for (tx, cost) { bank.cost_tracker_mutable().add(tx, cost)->result;
// for OK: tx into candidates_txs list;
// for err: tx into retryable_txs;
Expand Down Expand Up @@ -1155,7 +1154,7 @@ impl BankingStage {
&tx,
// TODO TAO - refactor transaction_cost to avoid vec<key>, use index[]
// instead; so it can be readGuard
cost_model
&cost_model
.read()
.unwrap()
.calculate_cost(&tx, demote_program_write_locks),
Expand Down Expand Up @@ -1288,18 +1287,15 @@ impl BankingStage {
if unprocessed_tx_indexes.iter().all(|&i| i != index) {
bank.write_cost_tracker().unwrap().add_transaction_cost(
tx,
// TODO TAO - refactor transaction_cost to avoid vec<key>, use index[]
// instead; so it can be readGuard
// TODO TAO - refactor to avoid second cost_model calculation. Should cache
// first calc results and re-use, also need to resue later for bankless when to
// convert to fee to check account balance.
cost_model
.read()
.unwrap()
.calculate_cost(
tx,
bank.demote_program_write_locks()
),
// TODO TAO - refactor transaction_cost to avoid vec<key>, use index[]
// instead; so it can be readGuard
// TODO TAO - refactor to avoid second cost_model calculation. Should cache
// first calc results and re-use, also need to resue later for bankless when to
// convert to fee to check account balance.
&cost_model
.read()
.unwrap()
.calculate_cost(tx, bank.demote_program_write_locks()),
cost_tracker_stats,
);
}
Expand Down Expand Up @@ -1756,7 +1752,7 @@ mod tests {
gossip_verified_vote_receiver,
None,
gossip_vote_sender,
&Arc::new(RwLock::new(CostModel::default())),
Arc::new(RwLock::new(CostModel::default())),
);
drop(verified_sender);
drop(gossip_verified_vote_sender);
Expand Down Expand Up @@ -1805,7 +1801,7 @@ mod tests {
verified_gossip_vote_receiver,
None,
gossip_vote_sender,
&Arc::new(RwLock::new(CostModel::default())),
Arc::new(RwLock::new(CostModel::default())),
);
trace!("sending bank");
drop(verified_sender);
Expand Down Expand Up @@ -1878,7 +1874,7 @@ mod tests {
gossip_verified_vote_receiver,
None,
gossip_vote_sender,
&Arc::new(RwLock::new(CostModel::default())),
Arc::new(RwLock::new(CostModel::default())),
);

// fund another account so we can send 2 good transactions in a single batch.
Expand Down Expand Up @@ -2029,7 +2025,7 @@ mod tests {
3,
None,
gossip_vote_sender,
&Arc::new(RwLock::new(CostModel::default())),
Arc::new(RwLock::new(CostModel::default())),
);

// wait for banking_stage to eat the packets
Expand Down Expand Up @@ -2828,9 +2824,9 @@ mod tests {
None::<Box<dyn Fn()>>,
&BankingStageStats::default(),
&recorder,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
&Arc::new(RwLock::new(
CostModel::default(),
))))),
)),
&mut CostTrackerStats::default(),
);
assert_eq!(buffered_packets[0].1.len(), num_conflicting_transactions);
Expand Down Expand Up @@ -2915,9 +2911,9 @@ mod tests {
test_fn,
&BankingStageStats::default(),
&recorder,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
&Arc::new(RwLock::new(
CostModel::default(),
))))),
)),
&mut CostTrackerStats::default(),
);

Expand Down Expand Up @@ -3177,6 +3173,7 @@ mod tests {
&BankingStageStats::default(),
false,
votes_only,
&Arc::new(RwLock::new(CostModel::default())),
&mut CostTrackerStats::default(),
);
assert_eq!(2, txs.len());
Expand All @@ -3192,6 +3189,7 @@ mod tests {
&BankingStageStats::default(),
false,
votes_only,
&Arc::new(RwLock::new(CostModel::default())),
&mut CostTrackerStats::default(),
);
assert_eq!(0, txs.len());
Expand All @@ -3216,6 +3214,7 @@ mod tests {
&BankingStageStats::default(),
false,
votes_only,
&Arc::new(RwLock::new(CostModel::default())),
&mut CostTrackerStats::default(),
);
assert_eq!(3, txs.len());
Expand All @@ -3231,6 +3230,7 @@ mod tests {
&BankingStageStats::default(),
false,
votes_only,
&Arc::new(RwLock::new(CostModel::default())),
&mut CostTrackerStats::default(),
);
assert_eq!(2, txs.len());
Expand All @@ -3255,6 +3255,7 @@ mod tests {
&BankingStageStats::default(),
false,
votes_only,
&Arc::new(RwLock::new(CostModel::default())),
&mut CostTrackerStats::default(),
);
assert_eq!(3, txs.len());
Expand All @@ -3270,6 +3271,7 @@ mod tests {
&BankingStageStats::default(),
false,
votes_only,
&Arc::new(RwLock::new(CostModel::default())),
&mut CostTrackerStats::default(),
);
assert_eq!(3, txs.len());
Expand Down
6 changes: 2 additions & 4 deletions ledger-tool/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -774,13 +774,11 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String>
let mut program_ids = HashMap::new();
let mut cost_model = CostModel::default();
cost_model.initialize_cost_table(&blockstore.read_program_costs().unwrap());
let cost_model = Arc::new(RwLock::new(cost_model));
let mut cost_tracker = CostTracker::new(cost_model.clone());
let mut cost_tracker = CostTracker::default();
let mut cost_tracker_stats = CostTrackerStats::default();

for entry in entries {
num_transactions += entry.transactions.len();
let mut cost_model = cost_model.write().unwrap();
entry
.transactions
.into_iter()
Expand All @@ -801,7 +799,7 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String>
true, // demote_program_write_locks
);
if cost_tracker
.try_add(tx_cost, &mut cost_tracker_stats)
.try_add(&transaction, &tx_cost, &mut cost_tracker_stats)
.is_err()
{
println!(
Expand Down
43 changes: 20 additions & 23 deletions runtime/src/cost_model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,12 @@ impl TransactionCost {
}
}

// TODO TAO - does CostMOdel needs AbiExample?
#[derive(AbiExample, Debug)]
pub struct CostModel {
account_cost_limit: u64,
block_cost_limit: u64,
instruction_execution_cost_table: ExecuteCostTable,

// reusable variables
transaction_cost: TransactionCost,
}

impl Default for CostModel {
Expand All @@ -58,12 +56,11 @@ impl Default for CostModel {
}

impl CostModel {
pub fn new(chain_max: u64, block_max: u64) -> Self {
pub fn new(account_max: u64, block_max: u64) -> Self {
Self {
account_cost_limit: chain_max,
account_cost_limit: account_max,
block_cost_limit: block_max,
instruction_execution_cost_table: ExecuteCostTable::default(),
transaction_cost: TransactionCost::new_with_capacity(MAX_WRITABLE_ACCOUNTS),
}
}

Expand Down Expand Up @@ -106,22 +103,19 @@ impl CostModel {
}

pub fn calculate_cost(
&mut self,
&self,
transaction: &SanitizedTransaction,
demote_program_write_locks: bool,
) -> &TransactionCost {
self.transaction_cost.reset();
) -> TransactionCost {
let mut tx_cost = TransactionCost::new_with_capacity(MAX_WRITABLE_ACCOUNTS);

self.transaction_cost.signature_cost = self.get_signature_cost(transaction);
self.get_write_lock_cost(transaction, demote_program_write_locks);
self.transaction_cost.data_bytes_cost = self.get_data_bytes_cost(transaction);
self.transaction_cost.execution_cost = self.get_transaction_cost(transaction);
tx_cost.signature_cost = self.get_signature_cost(transaction);
self.get_write_lock_cost(&mut tx_cost, transaction, demote_program_write_locks);
tx_cost.data_bytes_cost = self.get_data_bytes_cost(transaction);
tx_cost.execution_cost = self.get_transaction_cost(transaction);

debug!(
"transaction {:?} has cost {:?}",
transaction, self.transaction_cost
);
&self.transaction_cost
debug!("transaction {:?} has cost {:?}", transaction, tx_cost);
tx_cost
}

pub fn upsert_instruction_cost(
Expand All @@ -146,17 +140,20 @@ impl CostModel {
}

fn get_write_lock_cost(
&mut self,
&self,
tx_cost: &mut TransactionCost,
transaction: &SanitizedTransaction,
demote_program_write_locks: bool,
) {
let message = transaction.message();
message.account_keys_iter().enumerate().for_each(|(i, k)| {
let is_writable = message.is_writable(i, demote_program_write_locks);

// TODO TAO - is pushing key really this expensive? Test it, otherwise,
// can push index
if is_writable {
self.transaction_cost.writable_accounts.push(*k);
self.transaction_cost.write_lock_cost += WRITE_LOCK_UNITS;
tx_cost.writable_accounts.push(*k);
tx_cost.write_lock_cost += WRITE_LOCK_UNITS;
}
});
}
Expand Down Expand Up @@ -368,7 +365,7 @@ mod tests {
.try_into()
.unwrap();

let mut cost_model = CostModel::default();
let cost_model = CostModel::default();
let tx_cost = cost_model.calculate_cost(&tx, /*demote_program_write_locks=*/ true);
assert_eq!(2 + 2, tx_cost.writable_accounts.len());
assert_eq!(signer1.pubkey(), tx_cost.writable_accounts[0]);
Expand Down Expand Up @@ -479,7 +476,7 @@ mod tests {
})
} else {
thread::spawn(move || {
let mut cost_model = cost_model.write().unwrap();
let cost_model = cost_model.write().unwrap();
let tx_cost = cost_model
.calculate_cost(&tx, /*demote_program_write_locks=*/ true);
assert_eq!(3, tx_cost.writable_accounts.len());
Expand Down
Loading

0 comments on commit b23420b

Please sign in to comment.