Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor cost tracking #1954

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 6 additions & 30 deletions core/benches/consumer.rs
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
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
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