From a3f85aba21f6c43608a64ddd22a2ac0312fdc5ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 13 Oct 2023 21:59:48 +0200 Subject: [PATCH] Refactor - LoadedPrograms part 2 (#33694) --- program-runtime/src/loaded_programs.rs | 248 +++++++++++++------------ runtime/src/bank.rs | 20 +- runtime/src/bank/tests.rs | 22 ++- 3 files changed, 157 insertions(+), 133 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 43061f19a..0ded17ee7 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -56,9 +56,12 @@ pub trait ForkGraph { /// Provides information about current working slot, and its ancestors pub trait WorkingSlot { - /// Returns the current slot value + /// Returns the current slot fn current_slot(&self) -> Slot; + /// Returns the epoch of the current slot + fn current_epoch(&self) -> Epoch; + /// Returns true if the `other` slot is an ancestor of self, false otherwise fn is_ancestor(&self, other: Slot) -> bool; } @@ -99,6 +102,20 @@ impl Debug for LoadedProgramType { } } +impl LoadedProgramType { + /// Returns a reference to its environment if it has one + pub fn get_environment(&self) -> Option<&ProgramRuntimeEnvironment> { + match self { + LoadedProgramType::LegacyV0(program) + | LoadedProgramType::LegacyV1(program) + | LoadedProgramType::Typed(program) => Some(program.get_loader()), + #[cfg(test)] + LoadedProgramType::TestLoaded(environment) => Some(environment), + _ => None, + } + } +} + #[derive(Debug, Default)] pub struct LoadedProgram { /// The program of this entry @@ -338,16 +355,8 @@ impl LoadedProgram { } pub fn to_unloaded(&self) -> Option { - let env = match &self.program { - LoadedProgramType::LegacyV0(program) - | LoadedProgramType::LegacyV1(program) - | LoadedProgramType::Typed(program) => program.get_loader().clone(), - #[cfg(test)] - LoadedProgramType::TestLoaded(env) => env.clone(), - _ => return None, - }; Some(Self { - program: LoadedProgramType::Unloaded(env), + program: LoadedProgramType::Unloaded(self.program.get_environment()?.clone()), account_size: self.account_size, deployment_slot: self.deployment_slot, effective_slot: self.effective_slot, @@ -523,6 +532,11 @@ pub enum LoadedProgramMatchCriteria { } impl LoadedPrograms { + /// Returns the current environments depending on the given epoch + pub fn get_environments_for_epoch(&self, _epoch: Epoch) -> &ProgramRuntimeEnvironments { + &self.environments + } + /// Refill the cache with a single entry. It's typically called during transaction loading, /// when the cache doesn't contain the entry corresponding to program `key`. /// The function dedupes the cache, in case some other thread replenished the entry in parallel. @@ -586,41 +600,13 @@ impl LoadedPrograms { pub fn prune_feature_set_transition(&mut self) { for second_level in self.entries.values_mut() { second_level.retain(|entry| { - let retain = match &entry.program { - LoadedProgramType::Builtin(_) - | LoadedProgramType::DelayVisibility - | LoadedProgramType::Closed => true, - LoadedProgramType::LegacyV0(program) | LoadedProgramType::LegacyV1(program) - if Arc::ptr_eq( - program.get_loader(), - &self.environments.program_runtime_v1, - ) => - { - true - } - LoadedProgramType::Unloaded(environment) - | LoadedProgramType::FailedVerification(environment) - if Arc::ptr_eq(environment, &self.environments.program_runtime_v1) - || Arc::ptr_eq(environment, &self.environments.program_runtime_v2) => - { - true - } - LoadedProgramType::Typed(program) - if Arc::ptr_eq( - program.get_loader(), - &self.environments.program_runtime_v2, - ) => - { - true - } - _ => false, - }; - if !retain { - self.stats - .prunes_environment - .fetch_add(1, Ordering::Relaxed); + if Self::matches_environment(entry, &self.environments) { + return true; } - retain + self.stats + .prunes_environment + .fetch_add(1, Ordering::Relaxed); + false }); } self.remove_programs_with_no_entries(); @@ -688,6 +674,17 @@ impl LoadedPrograms { } } + fn matches_environment( + entry: &Arc, + environments: &ProgramRuntimeEnvironments, + ) -> bool { + let Some(environment) = entry.program.get_environment() else { + return true; + }; + Arc::ptr_eq(environment, &environments.program_runtime_v1) + || Arc::ptr_eq(environment, &environments.program_runtime_v2) + } + fn matches_loaded_program_criteria( program: &Arc, criteria: &LoadedProgramMatchCriteria, @@ -727,6 +724,7 @@ impl LoadedPrograms { working_slot: &S, keys: impl Iterator, ) -> ExtractedPrograms { + let environments = self.get_environments_for_epoch(working_slot.current_epoch()); let mut missing = Vec::new(); let mut unloaded = Vec::new(); let found = keys @@ -738,27 +736,22 @@ impl LoadedPrograms { || entry.deployment_slot == current_slot || working_slot.is_ancestor(entry.deployment_slot) { - if !Self::is_entry_usable(entry, current_slot, &match_criteria) { - missing.push((key, count)); - return None; - } - - if let LoadedProgramType::Unloaded(environment) = &entry.program { - if Arc::ptr_eq(environment, &self.environments.program_runtime_v1) - || Arc::ptr_eq( - environment, - &self.environments.program_runtime_v2, - ) - { - // if the environment hasn't changed since the entry was unloaded. - unloaded.push((key, count)); - } else { - missing.push((key, count)); - } - return None; - } - if current_slot >= entry.effective_slot { + if !Self::is_entry_usable(entry, current_slot, &match_criteria) { + missing.push((key, count)); + return None; + } + + if !Self::matches_environment(entry, environments) { + missing.push((key, count)); + return None; + } + + if let LoadedProgramType::Unloaded(_environment) = &entry.program { + unloaded.push((key, count)); + return None; + } + let mut usage_count = entry.tx_usage_counter.load(Ordering::Relaxed); saturating_add_assign!(usage_count, count); @@ -794,7 +787,7 @@ impl LoadedPrograms { loaded: LoadedProgramsForTxBatch { entries: found, slot: working_slot.current_slot(), - environments: self.environments.clone(), + environments: environments.clone(), }, missing, unloaded, @@ -930,13 +923,16 @@ mod tests { use { crate::loaded_programs::{ BlockRelation, ExtractedPrograms, ForkGraph, LoadedProgram, LoadedProgramMatchCriteria, - LoadedProgramType, LoadedPrograms, LoadedProgramsForTxBatch, WorkingSlot, - DELAY_VISIBILITY_SLOT_OFFSET, + LoadedProgramType, LoadedPrograms, LoadedProgramsForTxBatch, ProgramRuntimeEnvironment, + WorkingSlot, DELAY_VISIBILITY_SLOT_OFFSET, }, assert_matches::assert_matches, percentage::Percentage, solana_rbpf::vm::BuiltinProgram, - solana_sdk::{clock::Slot, pubkey::Pubkey}, + solana_sdk::{ + clock::{Epoch, Slot}, + pubkey::Pubkey, + }, std::{ ops::ControlFlow, sync::{ @@ -946,6 +942,51 @@ mod tests { }, }; + static MOCK_ENVIRONMENT: std::sync::OnceLock = + std::sync::OnceLock::::new(); + + fn new_mock_cache() -> LoadedPrograms { + let mut cache = LoadedPrograms::default(); + cache.environments.program_runtime_v1 = MOCK_ENVIRONMENT + .get_or_init(|| Arc::new(BuiltinProgram::new_mock())) + .clone(); + cache + } + + fn new_test_loaded_program(deployment_slot: Slot, effective_slot: Slot) -> Arc { + new_test_loaded_program_with_usage(deployment_slot, effective_slot, AtomicU64::default()) + } + + fn new_test_loaded_program_with_usage( + deployment_slot: Slot, + effective_slot: Slot, + usage_counter: AtomicU64, + ) -> Arc { + new_test_loaded_program_with_usage_and_expiry( + deployment_slot, + effective_slot, + usage_counter, + None, + ) + } + + fn new_test_loaded_program_with_usage_and_expiry( + deployment_slot: Slot, + effective_slot: Slot, + usage_counter: AtomicU64, + expiry: Option, + ) -> Arc { + Arc::new(LoadedProgram { + program: LoadedProgramType::TestLoaded(MOCK_ENVIRONMENT.get().unwrap().clone()), + account_size: 0, + deployment_slot, + effective_slot, + maybe_expiration_slot: expiry, + tx_usage_counter: usage_counter, + ix_usage_counter: AtomicU64::default(), + }) + } + fn new_test_builtin_program(deployment_slot: Slot, effective_slot: Slot) -> Arc { Arc::new(LoadedProgram { program: LoadedProgramType::Builtin(BuiltinProgram::new_mock()), @@ -1011,7 +1052,7 @@ mod tests { let mut programs = vec![]; let mut num_total_programs: usize = 0; - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); let program1 = Pubkey::new_unique(); let program1_deployment_slots = [0, 10, 20]; @@ -1177,7 +1218,7 @@ mod tests { #[test] fn test_usage_count_of_unloaded_program() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); let program = Pubkey::new_unique(); let num_total_programs = 6; @@ -1229,7 +1270,7 @@ mod tests { #[test] fn test_replace_tombstones() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); let program1 = Pubkey::new_unique(); let env = Arc::new(BuiltinProgram::new_mock()); set_tombstone( @@ -1261,7 +1302,7 @@ mod tests { assert_eq!(tombstone.deployment_slot, 100); assert_eq!(tombstone.effective_slot, 100); - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); let program1 = Pubkey::new_unique(); let tombstone = set_tombstone( &mut cache, @@ -1321,7 +1362,7 @@ mod tests { #[test] fn test_prune_empty() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); let fork_graph = TestForkGraph { relation: BlockRelation::Unrelated, }; @@ -1332,7 +1373,7 @@ mod tests { cache.prune(&fork_graph, 10, 0); assert!(cache.entries.is_empty()); - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); let fork_graph = TestForkGraph { relation: BlockRelation::Ancestor, }; @@ -1343,7 +1384,7 @@ mod tests { cache.prune(&fork_graph, 10, 0); assert!(cache.entries.is_empty()); - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); let fork_graph = TestForkGraph { relation: BlockRelation::Descendant, }; @@ -1354,7 +1395,7 @@ mod tests { cache.prune(&fork_graph, 10, 0); assert!(cache.entries.is_empty()); - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); let fork_graph = TestForkGraph { relation: BlockRelation::Unknown, }; @@ -1443,6 +1484,10 @@ mod tests { self.slot } + fn current_epoch(&self) -> Epoch { + 0 + } + fn is_ancestor(&self, other: Slot) -> bool { self.fork .iter() @@ -1452,41 +1497,6 @@ mod tests { } } - fn new_test_loaded_program(deployment_slot: Slot, effective_slot: Slot) -> Arc { - new_test_loaded_program_with_usage(deployment_slot, effective_slot, AtomicU64::default()) - } - - fn new_test_loaded_program_with_usage( - deployment_slot: Slot, - effective_slot: Slot, - usage_counter: AtomicU64, - ) -> Arc { - new_test_loaded_program_with_usage_and_expiry( - deployment_slot, - effective_slot, - usage_counter, - None, - ) - } - - fn new_test_loaded_program_with_usage_and_expiry( - deployment_slot: Slot, - effective_slot: Slot, - usage_counter: AtomicU64, - expiry: Option, - ) -> Arc { - let env = Arc::new(BuiltinProgram::new_mock()); - Arc::new(LoadedProgram { - program: LoadedProgramType::TestLoaded(env), - account_size: 0, - deployment_slot, - effective_slot, - maybe_expiration_slot: expiry, - tx_usage_counter: usage_counter, - ix_usage_counter: AtomicU64::default(), - }) - } - fn match_slot( table: &LoadedProgramsForTxBatch, program: &Pubkey, @@ -1502,7 +1512,7 @@ mod tests { #[test] fn test_fork_extract_and_prune() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); // Fork graph created for the test // 0 @@ -1883,7 +1893,7 @@ mod tests { #[test] fn test_extract_using_deployment_slot() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); // Fork graph created for the test // 0 @@ -1968,7 +1978,7 @@ mod tests { #[test] fn test_extract_unloaded() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); // Fork graph created for the test // 0 @@ -2081,13 +2091,12 @@ mod tests { assert!(match_slot(&found, &program1, 20, 22)); assert!(missing.contains(&(program2, 1))); - assert!(missing.contains(&(program3, 1))); - assert!(unloaded.is_empty()); + assert!(unloaded.contains(&(program3, 1))); } #[test] fn test_prune_expired() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); // Fork graph created for the test // 0 @@ -2206,7 +2215,7 @@ mod tests { #[test] fn test_fork_prune_find_first_ancestor() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); // Fork graph created for the test // 0 @@ -2252,7 +2261,7 @@ mod tests { #[test] fn test_prune_by_deployment_slot() { - let mut cache = LoadedPrograms::default(); + let mut cache = new_mock_cache(); // Fork graph created for the test // 0 @@ -2371,6 +2380,7 @@ mod tests { #[test] fn test_usable_entries_for_slot() { + new_mock_cache(); let tombstone = Arc::new(LoadedProgram::new_tombstone(0, LoadedProgramType::Closed)); assert!(LoadedPrograms::is_entry_usable( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4bbd82557..5227db287 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -933,6 +933,10 @@ impl WorkingSlot for Bank { self.slot } + fn current_epoch(&self) -> Epoch { + self.epoch + } + fn is_ancestor(&self, other: Slot) -> bool { self.ancestors.contains_key(&other) } @@ -4675,12 +4679,8 @@ impl Bank { } pub fn load_program(&self, pubkey: &Pubkey, reload: bool) -> Arc { - let environments = self - .loaded_programs_cache - .read() - .unwrap() - .environments - .clone(); + let loaded_programs_cache = self.loaded_programs_cache.read().unwrap(); + let environments = loaded_programs_cache.get_environments_for_epoch(self.epoch); let mut load_program_metrics = LoadProgramMetrics { program_id: pubkey.to_string(), @@ -4773,20 +4773,22 @@ impl Bank { }) .unwrap_or(LoadedProgram::new_tombstone( self.slot, - LoadedProgramType::FailedVerification(environments.program_runtime_v2), + LoadedProgramType::FailedVerification( + environments.program_runtime_v2.clone(), + ), )); Ok(loaded_program) } ProgramAccountLoadResult::InvalidV4Program => Ok(LoadedProgram::new_tombstone( self.slot, - LoadedProgramType::FailedVerification(environments.program_runtime_v2), + LoadedProgramType::FailedVerification(environments.program_runtime_v2.clone()), )), } .unwrap_or_else(|_| { LoadedProgram::new_tombstone( self.slot, - LoadedProgramType::FailedVerification(environments.program_runtime_v1), + LoadedProgramType::FailedVerification(environments.program_runtime_v1.clone()), ) }); diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index ef1553d9a..58e44366f 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -7,12 +7,15 @@ use { *, }, crate::{ - accounts_background_service::{PrunedBanksRequestHandler, SendDroppedBankCallback}, + accounts_background_service::{ + AbsRequestSender, PrunedBanksRequestHandler, SendDroppedBankCallback, + }, bank::replace_account::{ replace_empty_account_with_upgradeable_program, replace_non_upgradeable_program_account, ReplaceAccountError, }, bank_client::BankClient, + bank_forks::BankForks, epoch_rewards_hasher::hash_rewards_into_partitions, genesis_utils::{ self, activate_all_features, activate_feature, bootstrap_validator_stake_lamports, @@ -12502,7 +12505,8 @@ fn test_runtime_feature_enable_with_program_cache() { genesis_config .accounts .remove(&feature_set::reject_callx_r10::id()); - let root_bank = Arc::new(Bank::new_for_tests(&genesis_config)); + let mut bank_forks = BankForks::new(Bank::new_for_tests(&genesis_config)); + let root_bank = bank_forks.root_bank(); // Test a basic transfer let amount = genesis_config.rent.minimum_balance(0); @@ -12532,7 +12536,7 @@ fn test_runtime_feature_enable_with_program_cache() { // Advance the bank so the next transaction can be submitted. goto_end_of_slot(root_bank.clone()); - let mut bank = new_from_parent(root_bank); + let bank = Arc::new(new_from_parent(root_bank)); // Compose second instruction using the same program with a different block hash let instruction2 = Instruction::new_with_bytes(program_keypair.pubkey(), &[], Vec::new()); @@ -12558,9 +12562,17 @@ fn test_runtime_feature_enable_with_program_cache() { &feature_set::reject_callx_r10::id(), &feature::create_account(&Feature { activated_at: None }, feature_account_balance), ); - bank.apply_feature_activations(ApplyFeatureActivationsCaller::NewFromParent, false); - // Execute after feature is enabled to check it was pruned and reverified. + // Reroot to call LoadedPrograms::prune() and end the current recompilation phase + goto_end_of_slot(bank.clone()); + bank_forks.insert(Arc::into_inner(bank).unwrap()); + let bank = bank_forks.working_bank(); + bank_forks.set_root(bank.slot, &AbsRequestSender::default(), None); + + // Advance to next epoch, which starts the next recompilation phase + let bank = new_from_parent_next_epoch(bank, 1); + + // Execute after feature is enabled to check it was filtered out and reverified. let result_with_feature_enabled = bank.process_transaction(&transaction2); assert_eq!( result_with_feature_enabled,