From 070a5a36b826c7c9252b705ebe4aef8ab843e89d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 7 Feb 2024 02:38:21 +0100 Subject: [PATCH] Cleanup - Removes `LoadedProgram::maybe_expiration_slot` (#35023) Removes LoadedProgram::maybe_expiration_slot. --- program-runtime/src/loaded_programs.rs | 385 +++++-------------------- programs/bpf_loader/src/lib.rs | 4 - programs/loader-v4/src/lib.rs | 2 - svm/src/transaction_processor.rs | 2 - 4 files changed, 74 insertions(+), 319 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index e8e3b9ee3..19f5f7486 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -137,8 +137,6 @@ pub struct LoadedProgram { pub deployment_slot: Slot, /// Slot in which this entry will become active (can be in the future) pub effective_slot: Slot, - /// Optional expiration slot for this entry, after which it is treated as non-existent - pub maybe_expiration_slot: Option, /// How often this entry was used by a transaction pub tx_usage_counter: AtomicU64, /// How often this entry was used by an instruction @@ -282,7 +280,6 @@ impl LoadedProgram { program_runtime_environment: ProgramRuntimeEnvironment, deployment_slot: Slot, effective_slot: Slot, - maybe_expiration_slot: Option, elf_bytes: &[u8], account_size: usize, metrics: &mut LoadProgramMetrics, @@ -292,7 +289,6 @@ impl LoadedProgram { program_runtime_environment, deployment_slot, effective_slot, - maybe_expiration_slot, elf_bytes, account_size, metrics, @@ -313,7 +309,6 @@ impl LoadedProgram { program_runtime_environment: Arc>>, deployment_slot: Slot, effective_slot: Slot, - maybe_expiration_slot: Option, elf_bytes: &[u8], account_size: usize, metrics: &mut LoadProgramMetrics, @@ -323,7 +318,6 @@ impl LoadedProgram { program_runtime_environment, deployment_slot, effective_slot, - maybe_expiration_slot, elf_bytes, account_size, metrics, @@ -336,7 +330,6 @@ impl LoadedProgram { program_runtime_environment: Arc>>, deployment_slot: Slot, effective_slot: Slot, - maybe_expiration_slot: Option, elf_bytes: &[u8], account_size: usize, metrics: &mut LoadProgramMetrics, @@ -381,7 +374,6 @@ impl LoadedProgram { deployment_slot, account_size, effective_slot, - maybe_expiration_slot, tx_usage_counter: AtomicU64::new(0), program, ix_usage_counter: AtomicU64::new(0), @@ -395,7 +387,6 @@ impl LoadedProgram { account_size: self.account_size, deployment_slot: self.deployment_slot, effective_slot: self.effective_slot, - maybe_expiration_slot: self.maybe_expiration_slot, tx_usage_counter: AtomicU64::new(self.tx_usage_counter.load(Ordering::Relaxed)), ix_usage_counter: AtomicU64::new(self.ix_usage_counter.load(Ordering::Relaxed)), latest_access_slot: AtomicU64::new(self.latest_access_slot.load(Ordering::Relaxed)), @@ -416,7 +407,6 @@ impl LoadedProgram { deployment_slot, account_size, effective_slot: deployment_slot, - maybe_expiration_slot: None, tx_usage_counter: AtomicU64::new(0), program: LoadedProgramType::Builtin(BuiltinProgram::new_builtin(function_registry)), ix_usage_counter: AtomicU64::new(0), @@ -425,14 +415,11 @@ impl LoadedProgram { } pub fn new_tombstone(slot: Slot, reason: LoadedProgramType) -> Self { - let maybe_expiration_slot = matches!(reason, LoadedProgramType::DelayVisibility) - .then_some(slot.saturating_add(DELAY_VISIBILITY_SLOT_OFFSET)); let tombstone = Self { program: reason, account_size: 0, deployment_slot: slot, effective_slot: slot, - maybe_expiration_slot, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::new(0), @@ -835,12 +822,6 @@ impl LoadedPrograms { } }) .filter(|entry| { - // Remove expired - if let Some(expiration) = entry.maybe_expiration_slot { - if expiration <= new_root_slot { - return false; - } - } // Remove outdated environment of previous feature set if recompilation_phase_ends && !Self::matches_environment(entry, &self.environments) @@ -885,25 +866,6 @@ impl LoadedPrograms { } } - fn is_entry_usable( - entry: &Arc, - current_slot: Slot, - match_criteria: &LoadedProgramMatchCriteria, - ) -> bool { - if entry - .maybe_expiration_slot - .map(|expiration_slot| expiration_slot <= current_slot) - .unwrap_or(false) - { - // Found an entry that's already expired. Any further entries in the list - // are older than the current one. So treat the program as missing in the - // cache and return early. - return false; - } - - Self::matches_loaded_program_criteria(entry, match_criteria) - } - /// Extracts a subset of the programs relevant to a transaction batch /// and returns which program accounts the accounts DB needs to load. pub fn extract( @@ -933,14 +895,9 @@ impl LoadedPrograms { entry, &loaded_programs_for_tx_batch.environments, ) { - if !Self::is_entry_usable( - entry, - loaded_programs_for_tx_batch.slot, - match_criteria, - ) { + if !Self::matches_loaded_program_criteria(entry, match_criteria) { break; } - if let LoadedProgramType::Unloaded(_environment) = &entry.program { break; } @@ -1246,27 +1203,12 @@ mod tests { 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(), latest_access_slot: AtomicU64::new(deployment_slot), @@ -1279,7 +1221,6 @@ mod tests { account_size: 0, deployment_slot, effective_slot, - maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::default(), @@ -1308,7 +1249,6 @@ mod tests { account_size: 0, deployment_slot: slot, effective_slot: slot.saturating_add(1), - maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::default(), @@ -1905,7 +1845,6 @@ mod tests { account_size: 0, deployment_slot: 20, effective_slot: 20, - maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::default(), @@ -2192,58 +2131,6 @@ mod tests { assert!(match_missing(&missing, &program3, false)); - // The following is a special case, where there's an expiration slot - let test_program = Arc::new(LoadedProgram { - program: LoadedProgramType::DelayVisibility, - account_size: 0, - deployment_slot: 19, - effective_slot: 19, - maybe_expiration_slot: Some(21), - tx_usage_counter: AtomicU64::default(), - ix_usage_counter: AtomicU64::default(), - latest_access_slot: AtomicU64::default(), - }); - assert!(!cache.replenish(program4, test_program).0); - - // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 - let mut missing = vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ]; - let mut extracted = LoadedProgramsForTxBatch::new(19, cache.environments.clone()); - cache.extract(&mut missing, &mut extracted, true); - - assert!(match_slot(&extracted, &program1, 0, 19)); - assert!(match_slot(&extracted, &program2, 11, 19)); - // Program4 deployed at slot 19 should not be expired yet - assert!(match_slot(&extracted, &program4, 19, 19)); - - assert!(match_missing(&missing, &program3, false)); - - // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 21 - // This would cause program4 deployed at slot 19 to be expired. - let mut missing = vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ]; - let mut extracted = LoadedProgramsForTxBatch::new(21, cache.environments.clone()); - cache.extract(&mut missing, &mut extracted, true); - - assert!(match_slot(&extracted, &program1, 0, 21)); - assert!(match_slot(&extracted, &program2, 11, 21)); - - assert!(match_missing(&missing, &program3, false)); - assert!(match_missing(&missing, &program4, false)); - - // Remove the expired entry to let the rest of the test continue - if let Some(second_level) = cache.entries.get_mut(&program4) { - second_level.slot_versions.pop(); - } - cache.prune(5, 0); // Fork graph after pruning @@ -2499,117 +2386,6 @@ mod tests { assert!(match_missing(&missing, &program3, true)); } - #[test] - fn test_prune_expired() { - let mut cache = new_mock_cache::(); - - // Fork graph created for the test - // 0 - // / \ - // 10 5 - // | | - // 20 11 - // | | \ - // 22 15 25 - // | | - // 16 27 - // | - // 19 - // | - // 23 - - let mut fork_graph = TestForkGraphSpecific::default(); - fork_graph.insert_fork(&[0, 10, 20, 22]); - fork_graph.insert_fork(&[0, 5, 11, 12, 15, 16, 18, 19, 21, 23]); - fork_graph.insert_fork(&[0, 5, 11, 25, 27]); - let fork_graph = Arc::new(RwLock::new(fork_graph)); - cache.set_fork_graph(fork_graph); - - let program1 = Pubkey::new_unique(); - assert!(!cache.replenish(program1, new_test_loaded_program(10, 11)).0); - assert!(!cache.replenish(program1, new_test_loaded_program(20, 21)).0); - - let program2 = Pubkey::new_unique(); - assert!(!cache.replenish(program2, new_test_loaded_program(5, 6)).0); - assert!(!cache.replenish(program2, new_test_loaded_program(11, 12)).0); - - let program3 = Pubkey::new_unique(); - assert!(!cache.replenish(program3, new_test_loaded_program(25, 26)).0); - - // The following is a special case, where there's an expiration slot - let test_program = Arc::new(LoadedProgram { - program: LoadedProgramType::DelayVisibility, - account_size: 0, - deployment_slot: 11, - effective_slot: 11, - maybe_expiration_slot: Some(15), - tx_usage_counter: AtomicU64::default(), - ix_usage_counter: AtomicU64::default(), - latest_access_slot: AtomicU64::default(), - }); - assert!(!cache.replenish(program1, test_program).0); - - // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 - let mut missing = vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ]; - let mut extracted = LoadedProgramsForTxBatch::new(12, cache.environments.clone()); - cache.extract(&mut missing, &mut extracted, true); - - // Program1 deployed at slot 11 should not be expired yet - assert!(match_slot(&extracted, &program1, 11, 12)); - assert!(match_slot(&extracted, &program2, 11, 12)); - - assert!(match_missing(&missing, &program3, false)); - - // Testing fork 0 - 5 - 11 - 12 - 15 - 16 - 19 - 21 - 23 with current slot at 15 - // This would cause program4 deployed at slot 15 to be expired. - let mut missing = vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ]; - let mut extracted = LoadedProgramsForTxBatch::new(15, cache.environments.clone()); - cache.extract(&mut missing, &mut extracted, true); - - assert!(match_slot(&extracted, &program2, 11, 15)); - - assert!(match_missing(&missing, &program1, false)); - assert!(match_missing(&missing, &program3, false)); - - // Test that the program still exists in the cache, even though it is expired. - assert_eq!( - cache - .entries - .get(&program1) - .expect("Didn't find program1") - .slot_versions - .len(), - 3 - ); - - // New root 5 should not evict the expired entry for program1 - cache.prune(5, 0); - assert_eq!( - cache - .entries - .get(&program1) - .expect("Didn't find program1") - .slot_versions - .len(), - 1 - ); - - // Unlock the cooperative loading lock so that the subsequent prune can do its job - cache.finish_cooperative_loading_task(15, program1, new_test_loaded_program(0, 1)); - - // New root 15 should evict the expired entry for program1 - cache.prune(15, 0); - assert!(cache.entries.get(&program1).is_none()); - } - #[test] fn test_fork_prune_find_first_ancestor() { let mut cache = new_mock_cache::(); @@ -2741,109 +2517,96 @@ mod tests { new_mock_cache::(); let tombstone = Arc::new(LoadedProgram::new_tombstone(0, LoadedProgramType::Closed)); - assert!(LoadedPrograms::::is_entry_usable( - &tombstone, - 0, - &LoadedProgramMatchCriteria::NoCriteria - )); + assert!( + LoadedPrograms::::matches_loaded_program_criteria( + &tombstone, + &LoadedProgramMatchCriteria::NoCriteria + ) + ); - assert!(LoadedPrograms::::is_entry_usable( - &tombstone, - 1, - &LoadedProgramMatchCriteria::Tombstone - )); + assert!( + LoadedPrograms::::matches_loaded_program_criteria( + &tombstone, + &LoadedProgramMatchCriteria::Tombstone + ) + ); - assert!(LoadedPrograms::::is_entry_usable( - &tombstone, - 1, - &LoadedProgramMatchCriteria::NoCriteria - )); + assert!( + LoadedPrograms::::matches_loaded_program_criteria( + &tombstone, + &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(0) + ) + ); - assert!(LoadedPrograms::::is_entry_usable( - &tombstone, - 1, - &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(0) - )); - - assert!(!LoadedPrograms::::is_entry_usable( - &tombstone, - 1, - &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(1) - )); + assert!( + !LoadedPrograms::::matches_loaded_program_criteria( + &tombstone, + &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(1) + ) + ); let program = new_test_loaded_program(0, 1); - assert!(LoadedPrograms::::is_entry_usable( - &program, - 0, - &LoadedProgramMatchCriteria::NoCriteria - )); + assert!( + LoadedPrograms::::matches_loaded_program_criteria( + &program, + &LoadedProgramMatchCriteria::NoCriteria + ) + ); - assert!(!LoadedPrograms::::is_entry_usable( - &program, - 1, - &LoadedProgramMatchCriteria::Tombstone - )); + assert!( + !LoadedPrograms::::matches_loaded_program_criteria( + &program, + &LoadedProgramMatchCriteria::Tombstone + ) + ); - assert!(LoadedPrograms::::is_entry_usable( - &program, - 1, - &LoadedProgramMatchCriteria::NoCriteria - )); + assert!( + LoadedPrograms::::matches_loaded_program_criteria( + &program, + &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(0) + ) + ); - assert!(LoadedPrograms::::is_entry_usable( - &program, - 1, - &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(0) - )); + assert!( + !LoadedPrograms::::matches_loaded_program_criteria( + &program, + &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(1) + ) + ); - assert!(!LoadedPrograms::::is_entry_usable( - &program, - 1, - &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(1) - )); - - let program = Arc::new(new_test_loaded_program_with_usage_and_expiry( + let program = Arc::new(new_test_loaded_program_with_usage( 0, 1, AtomicU64::default(), - Some(2), )); - assert!(LoadedPrograms::::is_entry_usable( - &program, - 0, - &LoadedProgramMatchCriteria::NoCriteria - )); + assert!( + LoadedPrograms::::matches_loaded_program_criteria( + &program, + &LoadedProgramMatchCriteria::NoCriteria + ) + ); - assert!(LoadedPrograms::::is_entry_usable( - &program, - 1, - &LoadedProgramMatchCriteria::NoCriteria - )); + assert!( + !LoadedPrograms::::matches_loaded_program_criteria( + &program, + &LoadedProgramMatchCriteria::Tombstone + ) + ); - assert!(!LoadedPrograms::::is_entry_usable( - &program, - 1, - &LoadedProgramMatchCriteria::Tombstone - )); + assert!( + LoadedPrograms::::matches_loaded_program_criteria( + &program, + &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(0) + ) + ); - assert!(!LoadedPrograms::::is_entry_usable( - &program, - 2, - &LoadedProgramMatchCriteria::NoCriteria - )); - - assert!(LoadedPrograms::::is_entry_usable( - &program, - 1, - &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(0) - )); - - assert!(!LoadedPrograms::::is_entry_usable( - &program, - 1, - &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(1) - )); + assert!( + !LoadedPrograms::::matches_loaded_program_criteria( + &program, + &LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(1) + ) + ); } } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 48d44b718..21a7b5fed 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -81,7 +81,6 @@ pub fn load_program_from_bytes( program_runtime_environment, deployment_slot, effective_slot, - None, programdata, account_size, load_program_metrics, @@ -93,7 +92,6 @@ pub fn load_program_from_bytes( program_runtime_environment, deployment_slot, effective_slot, - None, programdata, account_size, load_program_metrics, @@ -4004,7 +4002,6 @@ mod tests { account_size: 0, deployment_slot: 0, effective_slot: 0, - maybe_expiration_slot: None, tx_usage_counter: AtomicU64::new(100), ix_usage_counter: AtomicU64::new(100), latest_access_slot: AtomicU64::new(0), @@ -4045,7 +4042,6 @@ mod tests { account_size: 0, deployment_slot: 0, effective_slot: 0, - maybe_expiration_slot: None, tx_usage_counter: AtomicU64::new(100), ix_usage_counter: AtomicU64::new(100), latest_access_slot: AtomicU64::new(0), diff --git a/programs/loader-v4/src/lib.rs b/programs/loader-v4/src/lib.rs index 20b413d23..4764b23fe 100644 --- a/programs/loader-v4/src/lib.rs +++ b/programs/loader-v4/src/lib.rs @@ -419,7 +419,6 @@ pub fn process_instruction_deploy( .clone(), deployment_slot, effective_slot, - None, programdata, buffer.get_data().len(), &mut load_program_metrics, @@ -660,7 +659,6 @@ mod tests { .clone(), 0, 0, - None, programdata, account.data().len(), &mut load_program_metrics, diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index f16891a85..837dc5e7f 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -774,7 +774,6 @@ impl TransactionBatchProcessor { program_runtime_environment.clone(), deployment_slot, deployment_slot.saturating_add(DELAY_VISIBILITY_SLOT_OFFSET), - None, programdata, account_size, load_program_metrics, @@ -786,7 +785,6 @@ impl TransactionBatchProcessor { program_runtime_environment.clone(), deployment_slot, deployment_slot.saturating_add(DELAY_VISIBILITY_SLOT_OFFSET), - None, programdata, account_size, load_program_metrics,