Skip to content

Commit

Permalink
OwnedOrBorrowed for TransactionBatch (#2837)
Browse files Browse the repository at this point in the history
* OwnedOrBorrowed

* relax trait bound to SVMMessage

* update batch with indexes trait bound

* import
  • Loading branch information
apfitzge authored Sep 16, 2024
1 parent 699981f commit 65c8996
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 27 deletions.
18 changes: 10 additions & 8 deletions ledger/benches/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ use {
genesis_utils::{create_genesis_config, GenesisConfigInfo},
},
solana_runtime::{
bank::Bank, bank_forks::BankForks, prioritization_fee_cache::PrioritizationFeeCache,
transaction_batch::TransactionBatch,
bank::Bank,
bank_forks::BankForks,
prioritization_fee_cache::PrioritizationFeeCache,
transaction_batch::{OwnedOrBorrowed, TransactionBatch},
},
solana_sdk::{
account::{Account, ReadableAccount},
Expand All @@ -24,10 +26,7 @@ use {
transaction::SanitizedTransaction,
},
solana_timings::ExecuteTimings,
std::{
borrow::Cow,
sync::{Arc, RwLock},
},
std::sync::{Arc, RwLock},
test::Bencher,
};

Expand Down Expand Up @@ -136,8 +135,11 @@ fn bench_execute_batch(
let batches: Vec<_> = transactions
.chunks(batch_size)
.map(|txs| {
let mut batch =
TransactionBatch::new(vec![Ok(()); txs.len()], &bank, Cow::Borrowed(txs));
let mut batch = TransactionBatch::new(
vec![Ok(()); txs.len()],
&bank,
OwnedOrBorrowed::Borrowed(txs),
);
batch.set_needs_unlock(false);
TransactionBatchWithIndexes {
batch,
Expand Down
10 changes: 5 additions & 5 deletions ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use {
installed_scheduler_pool::BankWithScheduler,
prioritization_fee_cache::PrioritizationFeeCache,
runtime_config::RuntimeConfig,
transaction_batch::TransactionBatch,
transaction_batch::{OwnedOrBorrowed, TransactionBatch},
vote_sender_types::ReplayVoteSender,
},
solana_sdk::{
Expand All @@ -56,12 +56,11 @@ use {
transaction_commit_result::{TransactionCommitResult, TransactionCommitResultExtensions},
transaction_processor::ExecutionRecordingConfig,
},
solana_svm_transaction::svm_transaction::SVMTransaction,
solana_svm_transaction::svm_message::SVMMessage,
solana_timings::{report_execute_timings, ExecuteTimingType, ExecuteTimings},
solana_transaction_status::token_balances::TransactionTokenBalancesSet,
solana_vote::vote_account::VoteAccountsHashMap,
std::{
borrow::Cow,
collections::{HashMap, HashSet},
ops::Index,
path::PathBuf,
Expand All @@ -78,7 +77,7 @@ use {
#[cfg(feature = "dev-context-only-utils")]
use {qualifier_attr::qualifiers, solana_runtime::bank::HashOverrides};

pub struct TransactionBatchWithIndexes<'a, 'b, Tx: SVMTransaction + Clone> {
pub struct TransactionBatchWithIndexes<'a, 'b, Tx: SVMMessage> {
pub batch: TransactionBatch<'a, 'b, Tx>,
pub transaction_indexes: Vec<usize>,
}
Expand Down Expand Up @@ -443,7 +442,8 @@ fn rebatch_transactions<'a>(
) -> TransactionBatchWithIndexes<'a, 'a, SanitizedTransaction> {
let txs = &sanitized_txs[start..=end];
let results = &lock_results[start..=end];
let mut tx_batch = TransactionBatch::new(results.to_vec(), bank, Cow::from(txs));
let mut tx_batch =
TransactionBatch::new(results.to_vec(), bank, OwnedOrBorrowed::Borrowed(txs));
tx_batch.set_needs_unlock(false);

let transaction_indexes = transaction_indexes[start..=end].to_vec();
Expand Down
13 changes: 6 additions & 7 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use {
},
stakes::{InvalidCacheEntryReason, Stakes, StakesCache, StakesEnum},
status_cache::{SlotDelta, StatusCache},
transaction_batch::TransactionBatch,
transaction_batch::{OwnedOrBorrowed, TransactionBatch},
},
byteorder::{ByteOrder, LittleEndian},
dashmap::{DashMap, DashSet},
Expand Down Expand Up @@ -175,7 +175,6 @@ use {
solana_vote::vote_account::{VoteAccount, VoteAccountsHashMap},
solana_vote_program::vote_state::VoteState,
std::{
borrow::Cow,
collections::{HashMap, HashSet},
convert::TryFrom,
fmt,
Expand Down Expand Up @@ -3341,7 +3340,7 @@ impl Bank {
Ok(TransactionBatch::new(
lock_results,
self,
Cow::Owned(sanitized_txs),
OwnedOrBorrowed::Owned(sanitized_txs),
))
}

Expand All @@ -3355,7 +3354,7 @@ impl Bank {
.rc
.accounts
.lock_accounts(txs.iter(), tx_account_lock_limit);
TransactionBatch::new(lock_results, self, Cow::Borrowed(txs))
TransactionBatch::new(lock_results, self, OwnedOrBorrowed::Borrowed(txs))
}

/// Prepare a locked transaction batch from a list of sanitized transactions, and their cost
Expand All @@ -3372,7 +3371,7 @@ impl Bank {
transaction_results,
tx_account_lock_limit,
);
TransactionBatch::new(lock_results, self, Cow::Borrowed(transactions))
TransactionBatch::new(lock_results, self, OwnedOrBorrowed::Borrowed(transactions))
}

/// Prepare a transaction batch from a single transaction without locking accounts
Expand All @@ -3386,7 +3385,7 @@ impl Bank {
let mut batch = TransactionBatch::new(
vec![lock_result],
self,
Cow::Borrowed(slice::from_ref(transaction)),
OwnedOrBorrowed::Borrowed(slice::from_ref(transaction)),
);
batch.set_needs_unlock(false);
batch
Expand Down Expand Up @@ -6818,7 +6817,7 @@ impl Bank {
.rc
.accounts
.lock_accounts(sanitized_txs.iter(), transaction_account_lock_limit);
TransactionBatch::new(lock_results, self, Cow::Owned(sanitized_txs))
TransactionBatch::new(lock_results, self, OwnedOrBorrowed::Owned(sanitized_txs))
}

/// Set the initial accounts data size
Expand Down
30 changes: 23 additions & 7 deletions runtime/src/transaction_batch.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,37 @@
use {
crate::bank::Bank, solana_sdk::transaction::Result,
solana_svm_transaction::svm_transaction::SVMTransaction, std::borrow::Cow,
crate::bank::Bank, core::ops::Deref, solana_sdk::transaction::Result,
solana_svm_transaction::svm_message::SVMMessage,
};

pub enum OwnedOrBorrowed<'a, T> {
Owned(Vec<T>),
Borrowed(&'a [T]),
}

impl<T> Deref for OwnedOrBorrowed<'_, T> {
type Target = [T];

fn deref(&self) -> &Self::Target {
match self {
OwnedOrBorrowed::Owned(v) => v,
OwnedOrBorrowed::Borrowed(v) => v,
}
}
}

// Represents the results of trying to lock a set of accounts
pub struct TransactionBatch<'a, 'b, Tx: SVMTransaction + Clone> {
pub struct TransactionBatch<'a, 'b, Tx: SVMMessage> {
lock_results: Vec<Result<()>>,
bank: &'a Bank,
sanitized_txs: Cow<'b, [Tx]>,
sanitized_txs: OwnedOrBorrowed<'b, Tx>,
needs_unlock: bool,
}

impl<'a, 'b, Tx: SVMTransaction + Clone> TransactionBatch<'a, 'b, Tx> {
impl<'a, 'b, Tx: SVMMessage> TransactionBatch<'a, 'b, Tx> {
pub fn new(
lock_results: Vec<Result<()>>,
bank: &'a Bank,
sanitized_txs: Cow<'b, [Tx]>,
sanitized_txs: OwnedOrBorrowed<'b, Tx>,
) -> Self {
assert_eq!(lock_results.len(), sanitized_txs.len());
Self {
Expand Down Expand Up @@ -81,7 +97,7 @@ impl<'a, 'b, Tx: SVMTransaction + Clone> TransactionBatch<'a, 'b, Tx> {
}

// Unlock all locked accounts in destructor.
impl<'a, 'b, Tx: SVMTransaction + Clone> Drop for TransactionBatch<'a, 'b, Tx> {
impl<'a, 'b, Tx: SVMMessage> Drop for TransactionBatch<'a, 'b, Tx> {
fn drop(&mut self) {
if self.needs_unlock() {
self.set_needs_unlock(false);
Expand Down

0 comments on commit 65c8996

Please sign in to comment.