Skip to content

Commit

Permalink
Refactor cost tracking (anza-xyz#1954)
Browse files Browse the repository at this point in the history 
* Refactor and additional metrics for cost tracking (anza-xyz#1888)

* Refactor and add metrics:
- Combine remove_* and update_* functions to reduce locking on cost-tracker and iteration.
- Add method to calculate executed transaction cost by directly using actual execution cost and loaded accounts size;
- Wireup histogram to report loaded accounts size;
- Report time of block limits checking;
- Move account counters from ExecuteDetailsTimings to ExecuteAccountsDetails;

* Move committed transactions adjustment into its own function

* remove histogram for loaded accounts size due to performance impact
  • Loading branch information
@tao-stones
tao-stones authored Jul 2, 2024
1 parent f689ac1 commit f8630a3
Show file tree
Hide file tree
Showing 6 changed files with 256 additions and 181 deletions.
36 changes: 6 additions & 30 deletions core/benches/consumer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use {
solana_runtime::bank::Bank,
solana_sdk::{
account::{Account, ReadableAccount},
feature_set::apply_cost_tracker_during_replay,
signature::Keypair,
signer::Signer,
stake_history::Epoch,
Expand Down Expand Up @@ -97,7 +96,7 @@ struct BenchFrame {
signal_receiver: Receiver<(Arc<Bank>, (Entry, u64))>,
}

fn setup(apply_cost_tracker_during_replay: bool) -> BenchFrame {
fn setup() -> BenchFrame {
let mint_total = u64::MAX;
let GenesisConfigInfo {
mut genesis_config, ..
Expand All @@ -109,10 +108,6 @@ fn setup(apply_cost_tracker_during_replay: bool) -> BenchFrame {

let mut bank = Bank::new_for_benches(&genesis_config);

if !apply_cost_tracker_during_replay {
bank.deactivate_feature(&apply_cost_tracker_during_replay::id());
}

// Allow arbitrary transaction processing time for the purposes of this bench
bank.ns_per_slot = u128::MAX;

Expand All @@ -139,11 +134,7 @@ fn setup(apply_cost_tracker_during_replay: bool) -> BenchFrame {
}
}

fn bench_process_and_record_transactions(
bencher: &mut Bencher,
batch_size: usize,
apply_cost_tracker_during_replay: bool,
) {
fn bench_process_and_record_transactions(bencher: &mut Bencher, batch_size: usize) {
const TRANSACTIONS_PER_ITERATION: usize = 64;
assert_eq!(
TRANSACTIONS_PER_ITERATION % batch_size,
Expand All @@ -161,7 +152,7 @@ fn bench_process_and_record_transactions(
poh_recorder,
poh_service,
signal_receiver: _signal_receiver,
} = setup(apply_cost_tracker_during_replay);
} = setup();
let consumer = create_consumer(&poh_recorder);
let transactions = create_transactions(&bank, 2_usize.pow(20));
let mut transaction_iter = transactions.chunks(batch_size);
Expand All @@ -186,30 +177,15 @@ fn bench_process_and_record_transactions(

#[bench]
fn bench_process_and_record_transactions_unbatched(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 1, true);
bench_process_and_record_transactions(bencher, 1);
}

#[bench]
fn bench_process_and_record_transactions_half_batch(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 32, true);
bench_process_and_record_transactions(bencher, 32);
}

#[bench]
fn bench_process_and_record_transactions_full_batch(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 64, true);
}

#[bench]
fn bench_process_and_record_transactions_unbatched_disable_tx_cost_update(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 1, false);
}

#[bench]
fn bench_process_and_record_transactions_half_batch_disable_tx_cost_update(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 32, false);
}

#[bench]
fn bench_process_and_record_transactions_full_batch_disable_tx_cost_update(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 64, false);
bench_process_and_record_transactions(bencher, 64);
}
39 changes: 5 additions & 34 deletions core/src/banking_stage/consumer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,27 +518,14 @@ impl Consumer {

// Costs of all transactions are added to the cost_tracker before processing.
// To ensure accurate tracking of compute units, transactions that ultimately
// were not included in the block should have their cost removed.
QosService::remove_costs(
// were not included in the block should have their cost removed, the rest
// should update with their actually consumed units.
QosService::remove_or_update_costs(
transaction_qos_cost_results.iter(),
commit_transactions_result.as_ref().ok(),
bank,
);

// once feature `apply_cost_tracker_during_replay` is activated, leader shall no longer
// adjust block with executed cost (a behavior more inline with bankless leader), it
// should use requested, or default `compute_unit_limit` as transaction's execution cost.
if !bank
.feature_set
.is_active(&feature_set::apply_cost_tracker_during_replay::id())
{
QosService::update_costs(
transaction_qos_cost_results.iter(),
commit_transactions_result.as_ref().ok(),
bank,
);
}

retryable_transaction_indexes
.iter_mut()
.for_each(|x| *x += chunk_offset);
Expand Down Expand Up @@ -1432,16 +1419,6 @@ mod tests {

#[test]
fn test_bank_process_and_record_transactions_cost_tracker() {
for apply_cost_tracker_during_replay_enabled in [true, false] {
bank_process_and_record_transactions_cost_tracker(
apply_cost_tracker_during_replay_enabled,
);
}
}

fn bank_process_and_record_transactions_cost_tracker(
apply_cost_tracker_during_replay_enabled: bool,
) {
solana_logger::setup();
let GenesisConfigInfo {
genesis_config,
Expand All @@ -1450,9 +1427,6 @@ mod tests {
} = create_slow_genesis_config(10_000);
let mut bank = Bank::new_for_tests(&genesis_config);
bank.ns_per_slot = u128::MAX;
if !apply_cost_tracker_during_replay_enabled {
bank.deactivate_feature(&feature_set::apply_cost_tracker_during_replay::id());
}
let bank = bank.wrap_with_bank_forks_for_tests().0;
let pubkey = solana_sdk::pubkey::new_rand();

Expand Down Expand Up @@ -1521,8 +1495,7 @@ mod tests {

// TEST: it's expected that the allocation will execute but the transfer will not
// because of a shared write-lock between mint_keypair. Ensure only the first transaction
// takes compute units in the block AND the apply_cost_tracker_during_replay_enabled feature
// is applied correctly
// takes compute units in the block
let allocate_keypair = Keypair::new();
let transactions = sanitize_transactions(vec![
system_transaction::allocate(
Expand Down Expand Up @@ -1561,7 +1534,7 @@ mod tests {
);
assert_eq!(retryable_transaction_indexes, vec![1]);

let expected_block_cost = if !apply_cost_tracker_during_replay_enabled {
let expected_block_cost = {
let (actual_programs_execution_cost, actual_loaded_accounts_data_size_cost) =
match commit_transactions_result.first().unwrap() {
CommitTransactionDetails::Committed {
Expand All @@ -1587,8 +1560,6 @@ mod tests {
}

block_cost + cost.sum()
} else {
block_cost + CostModel::calculate_cost(&transactions[0], &bank.feature_set).sum()
};

assert_eq!(get_block_cost(), expected_block_cost);
Expand Down
126 changes: 43 additions & 83 deletions core/src/banking_stage/qos_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,39 +132,28 @@ impl QosService {
(select_results, num_included)
}

/// Updates the transaction costs for committed transactions. Does not handle removing costs
/// for transactions that didn't get recorded or committed
pub fn update_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
transaction_committed_status: Option<&Vec<CommitTransactionDetails>>,
bank: &Bank,
) {
if let Some(transaction_committed_status) = transaction_committed_status {
Self::update_committed_transaction_costs(
transaction_cost_results,
transaction_committed_status,
bank,
)
}
}

/// Removes transaction costs from the cost tracker if not committed or recorded
pub fn remove_costs<'a>(
/// Removes transaction costs from the cost tracker if not committed or recorded, or
/// updates the transaction costs for committed transactions.
pub fn remove_or_update_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
transaction_committed_status: Option<&Vec<CommitTransactionDetails>>,
bank: &Bank,
) {
match transaction_committed_status {
Some(transaction_committed_status) => Self::remove_uncommitted_transaction_costs(
transaction_cost_results,
transaction_committed_status,
bank,
),
None => Self::remove_transaction_costs(transaction_cost_results, bank),
Some(transaction_committed_status) => {
Self::remove_or_update_recorded_transaction_costs(
transaction_cost_results,
transaction_committed_status,
bank,
)
}
None => Self::remove_unrecorded_transaction_costs(transaction_cost_results, bank),
}
}

fn remove_uncommitted_transaction_costs<'a>(
/// For recorded transactions, remove units reserved by uncommitted transaction, or update
/// units for committed transactions.
fn remove_or_update_recorded_transaction_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
transaction_committed_status: &Vec<CommitTransactionDetails>,
bank: &Bank,
Expand All @@ -178,45 +167,31 @@ impl QosService {
// checked for update
if let Ok(tx_cost) = tx_cost {
num_included += 1;
if *transaction_committed_details == CommitTransactionDetails::NotCommitted {
cost_tracker.remove(tx_cost)
match transaction_committed_details {
CommitTransactionDetails::Committed {
compute_units,
loaded_accounts_data_size,
} => {
cost_tracker.update_execution_cost(
tx_cost,
*compute_units,
CostModel::calculate_loaded_accounts_data_size_cost(
*loaded_accounts_data_size,
&bank.feature_set,
),
);
}
CommitTransactionDetails::NotCommitted => {
cost_tracker.remove(tx_cost);
}
}
}
});
cost_tracker.sub_transactions_in_flight(num_included);
}

fn update_committed_transaction_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
transaction_committed_status: &Vec<CommitTransactionDetails>,
bank: &Bank,
) {
let mut cost_tracker = bank.write_cost_tracker().unwrap();
transaction_cost_results
.zip(transaction_committed_status)
.for_each(|(estimated_tx_cost, transaction_committed_details)| {
// Only transactions that the qos service included have to be
// checked for update
if let Ok(estimated_tx_cost) = estimated_tx_cost {
if let CommitTransactionDetails::Committed {
compute_units,
loaded_accounts_data_size,
} = transaction_committed_details
{
cost_tracker.update_execution_cost(
estimated_tx_cost,
*compute_units,
CostModel::calculate_loaded_accounts_data_size_cost(
*loaded_accounts_data_size,
&bank.feature_set,
),
)
}
}
});
}

fn remove_transaction_costs<'a>(
/// Remove reserved units for transaction batch that unsuccessfully recorded.
fn remove_unrecorded_transaction_costs<'a>(
transaction_cost_results: impl Iterator<Item = &'a transaction::Result<TransactionCost>>,
bank: &Bank,
) {
Expand Down Expand Up @@ -784,18 +759,11 @@ mod tests {
+ (execute_units_adjustment + loaded_accounts_data_size_cost_adjustment)
* transaction_count;

// All transactions are committed, no costs should be removed
QosService::remove_costs(qos_cost_results.iter(), Some(&committed_status), &bank);
assert_eq!(
total_txs_cost,
bank.read_cost_tracker().unwrap().block_cost()
);
assert_eq!(
transaction_count,
bank.read_cost_tracker().unwrap().transaction_count()
QosService::remove_or_update_costs(
qos_cost_results.iter(),
Some(&committed_status),
&bank,
);

QosService::update_costs(qos_cost_results.iter(), Some(&committed_status), &bank);
assert_eq!(
final_txs_cost,
bank.read_cost_tracker().unwrap().block_cost()
Expand Down Expand Up @@ -843,18 +811,7 @@ mod tests {
bank.read_cost_tracker().unwrap().block_cost()
);

// update costs doesn't impact non-committed
QosService::update_costs(qos_cost_results.iter(), None, &bank);
assert_eq!(
total_txs_cost,
bank.read_cost_tracker().unwrap().block_cost()
);
assert_eq!(
transaction_count,
bank.read_cost_tracker().unwrap().transaction_count()
);

QosService::remove_costs(qos_cost_results.iter(), None, &bank);
QosService::remove_or_update_costs(qos_cost_results.iter(), None, &bank);
assert_eq!(0, bank.read_cost_tracker().unwrap().block_cost());
assert_eq!(0, bank.read_cost_tracker().unwrap().transaction_count());
}
Expand Down Expand Up @@ -926,8 +883,11 @@ mod tests {
})
.collect();

QosService::remove_costs(qos_cost_results.iter(), Some(&committed_status), &bank);
QosService::update_costs(qos_cost_results.iter(), Some(&committed_status), &bank);
QosService::remove_or_update_costs(
qos_cost_results.iter(),
Some(&committed_status),
&bank,
);

// assert the final block cost
let mut expected_final_txs_count = 0u64;
Expand Down
Loading

0 comments on commit f8630a3

Please sign in to comment.