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

Ensure that uncommitted transactions are always removed from QoS #32285

Merged
38 changes: 32 additions & 6 deletions core/benches/consumer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![allow(clippy::integer_arithmetic)]
#![feature(test)]

use solana_sdk::feature_set::apply_cost_tracker_during_replay;
apfitzge marked this conversation as resolved.
Show resolved Hide resolved
use {
crossbeam_channel::{unbounded, Receiver},
rayon::{
Expand Down Expand Up @@ -31,6 +32,7 @@ use {
tempfile::TempDir,
test::Bencher,
};

extern crate test;

fn create_accounts(num: usize) -> Vec<Keypair> {
Expand Down Expand Up @@ -90,7 +92,7 @@ struct BenchFrame {
signal_receiver: Receiver<(Arc<Bank>, (Entry, u64))>,
}

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

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 @@ -127,15 +134,19 @@ fn setup() -> BenchFrame {
}
}

fn bench_process_and_record_transactions(bencher: &mut Bencher, batch_size: usize) {
fn bench_process_and_record_transactions(
bencher: &mut Bencher,
batch_size: usize,
apply_cost_tracker_during_replay: bool,
) {
let BenchFrame {
bank,
ledger_path: _ledger_path,
exit,
poh_recorder,
poh_service,
signal_receiver: _signal_receiver,
} = setup();
} = setup(apply_cost_tracker_during_replay);
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 @@ -155,15 +166,30 @@ fn bench_process_and_record_transactions(bencher: &mut Bencher, batch_size: usiz

#[bench]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes me wish there was a test_case-like crate for benches 😢
I guess these will only live until the feature is activated everywhere

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

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

#[bench]
fn bench_process_and_record_transactions_full_batch(bencher: &mut Bencher) {
bench_process_and_record_transactions(bencher, 64);
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);
}
2 changes: 1 addition & 1 deletion core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ impl Consumer {
..
} = execute_and_commit_transactions_output;

// Costs of all transactions are added before processing.
// 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(
tao-stones marked this conversation as resolved.
Show resolved Hide resolved
Expand Down