Skip to content

Commit

Permalink
Remove Rc and RefCell to carry program cache in invoke_context (solan…
Browse files Browse the repository at this point in the history 
…a-labs#31684)
  • Loading branch information
@pgarg66 @wen-coding
pgarg66 authored and wen-coding committed May 18, 2023
1 parent f86807f commit f9a47cd
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 96 deletions.
2 changes: 1 addition & 1 deletion ledger-tool/src/program.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
debug!("Loaded program {}", key);
loaded_programs.replenish(key, program);
}
invoke_context.programs_loaded_for_tx_batch = Rc::new(RefCell::new(loaded_programs));
invoke_context.programs_loaded_for_tx_batch = &loaded_programs;

invoke_context
.transaction_context
Expand Down
23 changes: 13 additions & 10 deletions program-runtime/src/invoke_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ pub struct InvokeContext<'a> {
current_compute_budget: ComputeBudget,
compute_meter: RefCell<u64>,
accounts_data_meter: AccountsDataMeter,
pub programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
pub programs_modified_by_tx: Rc<RefCell<LoadedProgramsForTxBatch>>,
pub programs_updated_only_for_global_cache: Rc<RefCell<LoadedProgramsForTxBatch>>,
pub programs_loaded_for_tx_batch: &'a LoadedProgramsForTxBatch,
pub programs_modified_by_tx: &'a mut LoadedProgramsForTxBatch,
pub programs_updated_only_for_global_cache: &'a mut LoadedProgramsForTxBatch,
pub feature_set: Arc<FeatureSet>,
pub timings: ExecuteDetailsTimings,
pub blockhash: Hash,
Expand All @@ -179,9 +179,9 @@ impl<'a> InvokeContext<'a> {
sysvar_cache: &'a SysvarCache,
log_collector: Option<Rc<RefCell<LogCollector>>>,
compute_budget: ComputeBudget,
programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
programs_modified_by_tx: Rc<RefCell<LoadedProgramsForTxBatch>>,
programs_updated_only_for_global_cache: Rc<RefCell<LoadedProgramsForTxBatch>>,
programs_loaded_for_tx_batch: &'a LoadedProgramsForTxBatch,
programs_modified_by_tx: &'a mut LoadedProgramsForTxBatch,
programs_updated_only_for_global_cache: &'a mut LoadedProgramsForTxBatch,
feature_set: Arc<FeatureSet>,
blockhash: Hash,
lamports_per_signature: u64,
Expand Down Expand Up @@ -905,7 +905,7 @@ macro_rules! with_mock_invoke_context_and_builtin_programs {
account::ReadableAccount, feature_set::FeatureSet, hash::Hash, sysvar::rent::Rent,
transaction_context::TransactionContext,
},
std::{cell::RefCell, rc::Rc, sync::Arc},
std::sync::Arc,
$crate::{
compute_budget::ComputeBudget, invoke_context::InvokeContext,
loaded_programs::LoadedProgramsForTxBatch, log_collector::LogCollector,
Expand Down Expand Up @@ -938,16 +938,19 @@ macro_rules! with_mock_invoke_context_and_builtin_programs {
}
}
});
let programs_loaded_for_tx_batch = LoadedProgramsForTxBatch::default();
let mut programs_modified_by_tx = LoadedProgramsForTxBatch::default();
let mut programs_updated_only_for_global_cache = LoadedProgramsForTxBatch::default();
let mut $invoke_context = InvokeContext::new(
&mut $transaction_context,
Rent::default(),
$builtin_programs,
&sysvar_cache,
Some(LogCollector::new_ref()),
compute_budget,
Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
&programs_loaded_for_tx_batch,
&mut programs_modified_by_tx,
&mut programs_updated_only_for_global_cache,
Arc::new(FeatureSet::all_enabled()),
Hash::default(),
0,
Expand Down
2 changes: 1 addition & 1 deletion program-runtime/src/loaded_programs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ pub struct LoadedPrograms {
pub stats: Stats,
}

#[derive(Debug, Default)]
#[derive(Clone, Debug, Default)]
pub struct LoadedProgramsForTxBatch {
/// Pubkey is the address of a program.
/// LoadedProgram is the corresponding program entry valid for the slot in which a transaction is being executed.
Expand Down
41 changes: 15 additions & 26 deletions programs/bpf_loader/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,8 @@ fn find_program_in_cache(
// the cache of the cache of the programs that are loaded for the transaction batch.
invoke_context
.programs_modified_by_tx
.borrow()
.find(pubkey)
.or_else(|| {
invoke_context
.programs_loaded_for_tx_batch
.borrow()
.find(pubkey)
})
.or_else(|| invoke_context.programs_loaded_for_tx_batch.find(pubkey))
}

macro_rules! deploy_program {
Expand All @@ -229,7 +223,7 @@ macro_rules! deploy_program {
$drop
load_program_metrics.program_id = $program_id.to_string();
load_program_metrics.submit_datapoint(&mut $invoke_context.timings);
$invoke_context.programs_modified_by_tx.borrow_mut().replenish($program_id, Arc::new(executor));
$invoke_context.programs_modified_by_tx.replenish($program_id, Arc::new(executor));
}};
}

Expand Down Expand Up @@ -1271,20 +1265,16 @@ fn process_loader_upgradeable_instruction(
.feature_set
.is_active(&delay_visibility_of_program_deployment::id())
{
invoke_context
.programs_modified_by_tx
.borrow_mut()
.replenish(
program_key,
Arc::new(LoadedProgram::new_tombstone(
clock.slot,
LoadedProgramType::Closed,
)),
);
invoke_context.programs_modified_by_tx.replenish(
program_key,
Arc::new(LoadedProgram::new_tombstone(
clock.slot,
LoadedProgramType::Closed,
)),
);
} else {
invoke_context
.programs_updated_only_for_global_cache
.borrow_mut()
.replenish(
program_key,
Arc::new(LoadedProgram::new_tombstone(
Expand Down Expand Up @@ -1714,9 +1704,12 @@ pub mod test_utils {
true,
false,
) {
let mut cache = invoke_context.programs_modified_by_tx.borrow_mut();
cache.set_slot_for_tests(DELAY_VISIBILITY_SLOT_OFFSET);
cache.replenish(*pubkey, Arc::new(loaded_program));
invoke_context
.programs_modified_by_tx
.set_slot_for_tests(DELAY_VISIBILITY_SLOT_OFFSET);
invoke_context
.programs_modified_by_tx
.replenish(*pubkey, Arc::new(loaded_program));
}
}
}
Expand Down Expand Up @@ -4109,7 +4102,6 @@ mod tests {
};
invoke_context
.programs_modified_by_tx
.borrow_mut()
.replenish(program_id, Arc::new(program));

assert!(matches!(
Expand All @@ -4119,7 +4111,6 @@ mod tests {

let updated_program = invoke_context
.programs_modified_by_tx
.borrow()
.find(&program_id)
.expect("Didn't find upgraded program in the cache");

Expand All @@ -4142,7 +4133,6 @@ mod tests {
};
invoke_context
.programs_modified_by_tx
.borrow_mut()
.replenish(program_id, Arc::new(program));

let program_id2 = Pubkey::new_unique();
Expand All @@ -4153,7 +4143,6 @@ mod tests {

let program2 = invoke_context
.programs_modified_by_tx
.borrow()
.find(&program_id2)
.expect("Didn't find upgraded program in the cache");

Expand Down
8 changes: 2 additions & 6 deletions runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1499,9 +1499,7 @@ mod tests {
},
std::{
borrow::Cow,
cell::RefCell,
convert::TryFrom,
rc::Rc,
sync::atomic::{AtomicBool, AtomicU64, Ordering},
thread, time,
},
Expand Down Expand Up @@ -1533,10 +1531,8 @@ mod tests {
executed_units: 0,
accounts_data_len_delta: 0,
},
programs_modified_by_tx: Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
programs_updated_only_for_global_cache: Rc::new(RefCell::new(
LoadedProgramsForTxBatch::default(),
)),
programs_modified_by_tx: Box::<LoadedProgramsForTxBatch>::default(),
programs_updated_only_for_global_cache: Box::<LoadedProgramsForTxBatch>::default(),
}
}

Expand Down
30 changes: 15 additions & 15 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ pub struct TransactionExecutionDetails {
pub enum TransactionExecutionResult {
Executed {
details: TransactionExecutionDetails,
programs_modified_by_tx: Rc<RefCell<LoadedProgramsForTxBatch>>,
programs_updated_only_for_global_cache: Rc<RefCell<LoadedProgramsForTxBatch>>,
programs_modified_by_tx: Box<LoadedProgramsForTxBatch>,
programs_updated_only_for_global_cache: Box<LoadedProgramsForTxBatch>,
},
NotExecuted(TransactionError),
}
Expand Down Expand Up @@ -4214,7 +4214,7 @@ impl Bank {
timings: &mut ExecuteTimings,
error_counters: &mut TransactionErrorMetrics,
log_messages_bytes_limit: Option<usize>,
programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
programs_loaded_for_tx_batch: &LoadedProgramsForTxBatch,
) -> TransactionExecutionResult {
let prev_accounts_data_len = self.load_accounts_data_size();
let transaction_accounts = std::mem::take(&mut loaded_transaction.accounts);
Expand Down Expand Up @@ -4264,10 +4264,8 @@ impl Bank {
let (blockhash, lamports_per_signature) = self.last_blockhash_and_lamports_per_signature();

let mut executed_units = 0u64;
let programs_modified_by_tx =
Rc::new(RefCell::new(LoadedProgramsForTxBatch::new(self.slot)));
let programs_updated_only_for_global_cache =
Rc::new(RefCell::new(LoadedProgramsForTxBatch::new(self.slot)));
let mut programs_modified_by_tx = LoadedProgramsForTxBatch::new(self.slot);
let mut programs_updated_only_for_global_cache = LoadedProgramsForTxBatch::new(self.slot);
let mut process_message_time = Measure::start("process_message_time");
let process_result = MessageProcessor::process_message(
&self.builtin_programs,
Expand All @@ -4277,8 +4275,8 @@ impl Bank {
self.rent_collector.rent,
log_collector.clone(),
programs_loaded_for_tx_batch,
programs_modified_by_tx.clone(),
programs_updated_only_for_global_cache.clone(),
&mut programs_modified_by_tx,
&mut programs_updated_only_for_global_cache,
self.feature_set.clone(),
compute_budget,
timings,
Expand Down Expand Up @@ -4382,8 +4380,10 @@ impl Bank {
executed_units,
accounts_data_len_delta,
},
programs_modified_by_tx,
programs_updated_only_for_global_cache,
programs_modified_by_tx: Box::new(programs_modified_by_tx),
programs_updated_only_for_global_cache: Box::new(
programs_updated_only_for_global_cache,
),
}
}

Expand Down Expand Up @@ -4603,7 +4603,7 @@ impl Bank {
timings,
&mut error_counters,
log_messages_bytes_limit,
programs_loaded_for_tx_batch.clone(),
&programs_loaded_for_tx_batch.borrow(),
);

if let TransactionExecutionResult::Executed {
Expand All @@ -4617,7 +4617,7 @@ impl Bank {
if details.status.is_ok() {
programs_loaded_for_tx_batch
.borrow_mut()
.merge(&programs_modified_by_tx.borrow());
.merge(programs_modified_by_tx);
}
}

Expand Down Expand Up @@ -5112,8 +5112,8 @@ impl Bank {
{
if details.status.is_ok() {
let mut cache = self.loaded_programs_cache.write().unwrap();
cache.merge(&programs_modified_by_tx.borrow());
cache.merge(&programs_updated_only_for_global_cache.borrow());
cache.merge(programs_modified_by_tx);
cache.merge(programs_updated_only_for_global_cache);
}
}
}
Expand Down
17 changes: 8 additions & 9 deletions runtime/src/bank/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,10 @@ use {
},
},
std::{
cell::RefCell,
collections::{HashMap, HashSet},
convert::{TryFrom, TryInto},
fs::File,
io::Read,
rc::Rc,
str::FromStr,
sync::{
atomic::{
Expand Down Expand Up @@ -175,10 +173,8 @@ fn new_execution_result(
executed_units: 0,
accounts_data_len_delta: 0,
},
programs_modified_by_tx: Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
programs_updated_only_for_global_cache: Rc::new(RefCell::new(
LoadedProgramsForTxBatch::default(),
)),
programs_modified_by_tx: Box::<LoadedProgramsForTxBatch>::default(),
programs_updated_only_for_global_cache: Box::<LoadedProgramsForTxBatch>::default(),
}
}

Expand Down Expand Up @@ -7604,9 +7600,12 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
Ok(()),
solana_bpf_loader_program::process_instruction,
|invoke_context| {
let mut cache = invoke_context.programs_modified_by_tx.borrow_mut();
cache.set_slot_for_tests(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET);
cache.replenish(program_keypair.pubkey(), loaded_program.clone());
invoke_context
.programs_modified_by_tx
.set_slot_for_tests(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET);
invoke_context
.programs_modified_by_tx
.replenish(program_keypair.pubkey(), loaded_program.clone());
},
|_invoke_context| {},
);
Expand Down
Loading

0 comments on commit f9a47cd

Please sign in to comment.