Skip to content

Commit

Permalink
simplify forward packet batches (#3642)
Browse files Browse the repository at this point in the history 
simplify forward packet batches - nonzero, no branching
  • Loading branch information
@apfitzge
apfitzge authored Nov 14, 2024
1 parent 8b4f5e0 commit 2cb11a7
Showing 1 changed file with 35 additions and 93 deletions.
128 changes: 35 additions & 93 deletions core/src/banking_stage/forward_packet_batches_by_accounts.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use {
super::immutable_deserialized_packet::ImmutableDeserializedPacket,
core::num::NonZeroU64,
solana_cost_model::{
block_cost_limits,
cost_model::CostModel,
cost_tracker::{CostTracker, UpdatedCosts},
transaction_cost::TransactionCost,
},
solana_feature_set::FeatureSet,
solana_perf::packet::Packet,
Expand Down Expand Up @@ -67,9 +67,8 @@ pub struct ForwardPacketBatchesByAccounts {
cost_tracker: CostTracker,

// Compute Unit limits for each batch
batch_vote_limit: u64,
batch_block_limit: u64,
batch_account_limit: u64,
batch_block_limit: NonZeroU64,
batch_account_limit: NonZeroU64,
}

impl ForwardPacketBatchesByAccounts {
Expand All @@ -82,22 +81,32 @@ impl ForwardPacketBatchesByAccounts {
.map(|_| ForwardBatch::default())
.collect();

let batch_vote_limit = block_cost_limits::MAX_VOTE_UNITS.saturating_div(limit_ratio as u64);
let batch_vote_limit =
NonZeroU64::new(block_cost_limits::MAX_VOTE_UNITS.saturating_div(limit_ratio as u64))
.expect("batch vote limit must not be zero");
let batch_block_limit =
block_cost_limits::MAX_BLOCK_UNITS.saturating_div(limit_ratio as u64);
let batch_account_limit =
block_cost_limits::MAX_WRITABLE_ACCOUNT_UNITS.saturating_div(limit_ratio as u64);
NonZeroU64::new(block_cost_limits::MAX_BLOCK_UNITS.saturating_div(limit_ratio as u64))
.expect("batch block limit must not be zero");
let batch_account_limit = NonZeroU64::new(
block_cost_limits::MAX_WRITABLE_ACCOUNT_UNITS.saturating_div(limit_ratio as u64),
)
.expect("batch account limit must not be zero");

let mut cost_tracker = CostTracker::default();
cost_tracker.set_limits(
batch_account_limit.saturating_mul(number_of_batches as u64),
batch_block_limit.saturating_mul(number_of_batches as u64),
batch_vote_limit.saturating_mul(number_of_batches as u64),
batch_account_limit
.get()
.saturating_mul(number_of_batches as u64),
batch_block_limit
.get()
.saturating_mul(number_of_batches as u64),
batch_vote_limit
.get()
.saturating_mul(number_of_batches as u64),
);
Self {
forward_batches,
cost_tracker,
batch_vote_limit,
batch_block_limit,
batch_account_limit,
}
Expand All @@ -112,7 +121,7 @@ impl ForwardPacketBatchesByAccounts {
let tx_cost = CostModel::calculate_cost(sanitized_transaction, feature_set);

if let Ok(updated_costs) = self.cost_tracker.try_add(&tx_cost) {
let batch_index = self.get_batch_index_by_updated_costs(&tx_cost, &updated_costs);
let batch_index = self.get_batch_index_by_updated_costs(&updated_costs);

if let Some(forward_batch) = self.forward_batches.get_mut(batch_index) {
forward_batch.forwardable_packets.push(immutable_packet);
Expand Down Expand Up @@ -144,24 +153,12 @@ impl ForwardPacketBatchesByAccounts {
// would be exceeded. Eg, if by block limit, it can be put into batch #1; by vote limit, it can
// be put into batch #2; and by account limit, it can be put into batch #3; then it should be
// put into batch #3 to satisfy all batch limits.
fn get_batch_index_by_updated_costs(
&self,
tx_cost: &TransactionCost<impl TransactionWithMeta>,
updated_costs: &UpdatedCosts,
) -> usize {
let Some(batch_index_by_block_limit) =
updated_costs.updated_block_cost.checked_div(match tx_cost {
TransactionCost::SimpleVote { .. } => self.batch_vote_limit,
TransactionCost::Transaction(_) => self.batch_block_limit,
})
else {
unreachable!("batch vote limit or block limit must not be zero")
};

fn get_batch_index_by_updated_costs(&self, updated_costs: &UpdatedCosts) -> usize {
let batch_index_by_block_cost =
updated_costs.updated_block_cost / self.batch_block_limit.get();
let batch_index_by_account_limit =
updated_costs.updated_costliest_account_cost / self.batch_account_limit;

batch_index_by_block_limit.max(batch_index_by_account_limit) as usize
updated_costs.updated_costliest_account_cost / self.batch_account_limit.get();
batch_index_by_block_cost.max(batch_index_by_account_limit) as usize
}
}

Expand All @@ -170,8 +167,6 @@ mod tests {
use {
super::*,
crate::banking_stage::unprocessed_packet_batches::DeserializedPacket,
lazy_static::lazy_static,
solana_cost_model::transaction_cost::{UsageCostDetails, WritableKeysTransaction},
solana_feature_set::FeatureSet,
solana_runtime_transaction::runtime_transaction::RuntimeTransaction,
solana_sdk::{
Expand Down Expand Up @@ -215,22 +210,6 @@ mod tests {
(sanitized_transaction, deserialized_packet, limit_ratio)
}

fn zero_transaction_cost() -> TransactionCost<'static, WritableKeysTransaction> {
lazy_static! {
static ref DUMMY_TRANSACTION: WritableKeysTransaction = WritableKeysTransaction(vec![]);
};

TransactionCost::Transaction(UsageCostDetails {
transaction: &DUMMY_TRANSACTION,
signature_cost: 0,
write_lock_cost: 0,
data_bytes_cost: 0,
programs_execution_cost: 0,
loaded_accounts_data_size_cost: 0,
allocated_accounts_data_size: 0,
})
}

#[test]
fn test_try_add_packet_to_multiple_batches() {
// setup two transactions, one has high priority that writes to hot account, the
Expand Down Expand Up @@ -370,49 +349,16 @@ mod tests {
fn test_get_batch_index_by_updated_costs() {
let test_cost = 99;

// check against vote limit only
{
let mut forward_packet_batches_by_accounts =
ForwardPacketBatchesByAccounts::new_with_default_batch_limits();
forward_packet_batches_by_accounts.batch_vote_limit = test_cost + 1;

let dummy_transaction = WritableKeysTransaction(vec![]);
let transaction_cost = TransactionCost::SimpleVote {
transaction: &dummy_transaction,
};
assert_eq!(
0,
forward_packet_batches_by_accounts.get_batch_index_by_updated_costs(
&transaction_cost,
&UpdatedCosts {
updated_block_cost: test_cost,
updated_costliest_account_cost: 0
}
)
);
assert_eq!(
1,
forward_packet_batches_by_accounts.get_batch_index_by_updated_costs(
&transaction_cost,
&UpdatedCosts {
updated_block_cost: test_cost + 1,
updated_costliest_account_cost: 0
}
)
);
}

// check against block limit only
{
let mut forward_packet_batches_by_accounts =
ForwardPacketBatchesByAccounts::new_with_default_batch_limits();
forward_packet_batches_by_accounts.batch_block_limit = test_cost + 1;
forward_packet_batches_by_accounts.batch_block_limit =
NonZeroU64::new(test_cost + 1).unwrap();

let transaction_cost = zero_transaction_cost();
assert_eq!(
0,
forward_packet_batches_by_accounts.get_batch_index_by_updated_costs(
&transaction_cost,
&UpdatedCosts {
updated_block_cost: test_cost,
updated_costliest_account_cost: 0
Expand All @@ -422,7 +368,6 @@ mod tests {
assert_eq!(
1,
forward_packet_batches_by_accounts.get_batch_index_by_updated_costs(
&transaction_cost,
&UpdatedCosts {
updated_block_cost: test_cost + 1,
updated_costliest_account_cost: 0
Expand All @@ -435,13 +380,12 @@ mod tests {
{
let mut forward_packet_batches_by_accounts =
ForwardPacketBatchesByAccounts::new_with_default_batch_limits();
forward_packet_batches_by_accounts.batch_account_limit = test_cost + 1;
forward_packet_batches_by_accounts.batch_account_limit =
NonZeroU64::new(test_cost + 1).unwrap();

let transaction_cost = zero_transaction_cost();
assert_eq!(
0,
forward_packet_batches_by_accounts.get_batch_index_by_updated_costs(
&transaction_cost,
&UpdatedCosts {
updated_block_cost: 0,
updated_costliest_account_cost: test_cost
Expand All @@ -451,7 +395,6 @@ mod tests {
assert_eq!(
1,
forward_packet_batches_by_accounts.get_batch_index_by_updated_costs(
&transaction_cost,
&UpdatedCosts {
updated_block_cost: 0,
updated_costliest_account_cost: test_cost + 1
Expand All @@ -467,15 +410,14 @@ mod tests {
{
let mut forward_packet_batches_by_accounts =
ForwardPacketBatchesByAccounts::new_with_default_batch_limits();
forward_packet_batches_by_accounts.batch_block_limit = test_cost + 1;
forward_packet_batches_by_accounts.batch_vote_limit = test_cost / 2 + 1;
forward_packet_batches_by_accounts.batch_account_limit = test_cost / 3 + 1;
forward_packet_batches_by_accounts.batch_block_limit =
NonZeroU64::new(test_cost + 1).unwrap();
forward_packet_batches_by_accounts.batch_account_limit =
NonZeroU64::new(test_cost / 3 + 1).unwrap();

let transaction_cost = zero_transaction_cost();
assert_eq!(
2,
forward_packet_batches_by_accounts.get_batch_index_by_updated_costs(
&transaction_cost,
&UpdatedCosts {
updated_block_cost: test_cost,
updated_costliest_account_cost: test_cost
Expand Down

0 comments on commit 2cb11a7

Please sign in to comment.