From 6dd43dd5f6ecf79b1149bf999efe3a59239c4480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 11 Dec 2023 15:19:29 +0000 Subject: [PATCH 1/9] Disables verification-less reloading. --- program-runtime/src/loaded_programs.rs | 14 ++++---------- runtime/src/bank.rs | 4 ++-- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 75bf3a3c0f2ea7..8ff270001b23f6 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -484,7 +484,7 @@ pub struct LoadedProgramsForTxBatch { pub struct ExtractedPrograms { pub loaded: LoadedProgramsForTxBatch, - pub missing: HashMap, + pub missing: HashMap, } impl LoadedProgramsForTxBatch { @@ -789,7 +789,6 @@ impl LoadedPrograms { let mut extracting = extracted.lock().unwrap(); extracting.loaded.entries = keys .filter_map(|(key, (match_criteria, usage_count))| { - let mut reloading = false; if let Some(second_level) = self.entries.get(&key) { for entry in second_level.iter().rev() { let is_ancestor = matches!( @@ -809,7 +808,6 @@ impl LoadedPrograms { } if let LoadedProgramType::Unloaded(_environment) = &entry.program { - reloading = true; break; } @@ -832,7 +830,7 @@ impl LoadedPrograms { } } } - extracting.missing.insert(key, (usage_count, reloading)); + extracting.missing.insert(key, usage_count); None }) .collect::>>(); @@ -1591,14 +1589,10 @@ mod tests { fn match_missing( extracted: &Arc>, key: &Pubkey, - reload: bool, + _reload: bool, ) -> bool { let extracted = extracted.lock().unwrap(); - extracted - .missing - .get(key) - .filter(|(_count, reloading)| *reloading == reload) - .is_some() + extracted.missing.get(key).is_some() } #[test] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1b1c0cefbd6504..a552eff46051e9 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5029,8 +5029,8 @@ impl Bank { // Load missing programs while global cache is unlocked let missing_programs: Vec<(Pubkey, Arc)> = missing .iter() - .map(|(key, (count, reloading))| { - let program = self.load_program(key, *reloading, None); + .map(|(key, count)| { + let program = self.load_program(key, false, None); program.tx_usage_counter.store(*count, Ordering::Relaxed); (*key, program) }) From 933a986c9fd5cccdcc7c12958a52c541df2df9e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 11 Dec 2023 15:50:03 +0000 Subject: [PATCH 2/9] Refactors LoadedPrograms::extract() to use a mutable parameter instead of returning the list of missing entries. --- program-runtime/src/loaded_programs.rs | 559 +++++++++++-------------- runtime/src/bank.rs | 27 +- 2 files changed, 243 insertions(+), 343 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 8ff270001b23f6..50bad6260d7373 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -25,7 +25,7 @@ use { fmt::{Debug, Formatter}, sync::{ atomic::{AtomicU64, Ordering}, - Arc, Mutex, RwLock, + Arc, RwLock, }, }, }; @@ -482,11 +482,6 @@ pub struct LoadedProgramsForTxBatch { pub environments: ProgramRuntimeEnvironments, } -pub struct ExtractedPrograms { - pub loaded: LoadedProgramsForTxBatch, - pub missing: HashMap, -} - impl LoadedProgramsForTxBatch { pub fn new(slot: Slot, environments: ProgramRuntimeEnvironments) -> Self { Self { @@ -772,78 +767,70 @@ impl LoadedPrograms { pub fn extract( &self, current_slot: Slot, - keys: impl Iterator, - ) -> Arc> { + search_for: &mut Vec<(Pubkey, (LoadedProgramMatchCriteria, u64))>, + ) -> LoadedProgramsForTxBatch { debug_assert!(self.fork_graph.is_some()); let locked_fork_graph = self.fork_graph.as_ref().unwrap().read().unwrap(); let current_epoch = locked_fork_graph.slot_epoch(current_slot).unwrap(); let environments = self.get_environments_for_epoch(current_epoch); - let extracted = Arc::new(Mutex::new(ExtractedPrograms { - loaded: LoadedProgramsForTxBatch { - entries: HashMap::new(), - slot: current_slot, - environments: environments.clone(), - }, - missing: HashMap::new(), - })); - let mut extracting = extracted.lock().unwrap(); - extracting.loaded.entries = keys - .filter_map(|(key, (match_criteria, usage_count))| { - if let Some(second_level) = self.entries.get(&key) { - for entry in second_level.iter().rev() { - let is_ancestor = matches!( - locked_fork_graph.relationship(entry.deployment_slot, current_slot), - BlockRelation::Ancestor - ); - - if entry.deployment_slot <= self.latest_root_slot - || entry.deployment_slot == current_slot - || is_ancestor - { - let entry_to_return = if current_slot >= entry.effective_slot { - if !Self::is_entry_usable(entry, current_slot, &match_criteria) - || !Self::matches_environment(entry, environments) - { - break; - } + let mut loaded = HashMap::new(); + search_for.retain(|(key, (match_criteria, usage_count))| { + if let Some(second_level) = self.entries.get(&key) { + for entry in second_level.iter().rev() { + let is_ancestor = matches!( + locked_fork_graph.relationship(entry.deployment_slot, current_slot), + BlockRelation::Ancestor + ); - if let LoadedProgramType::Unloaded(_environment) = &entry.program { - break; - } + if entry.deployment_slot <= self.latest_root_slot + || entry.deployment_slot == current_slot + || is_ancestor + { + let entry_to_return = if current_slot >= entry.effective_slot { + if !Self::is_entry_usable(entry, current_slot, &match_criteria) + || !Self::matches_environment(entry, environments) + { + break; + } - entry.clone() - } else if entry.is_implicit_delay_visibility_tombstone(current_slot) { - // Found a program entry on the current fork, but it's not effective - // yet. It indicates that the program has delayed visibility. Return - // the tombstone to reflect that. - Arc::new(LoadedProgram::new_tombstone( - entry.deployment_slot, - LoadedProgramType::DelayVisibility, - )) - } else { - continue; - }; - entry_to_return - .tx_usage_counter - .fetch_add(usage_count, Ordering::Relaxed); - return Some((key, entry_to_return)); - } + if let LoadedProgramType::Unloaded(_environment) = &entry.program { + break; + } + + entry.clone() + } else if entry.is_implicit_delay_visibility_tombstone(current_slot) { + // Found a program entry on the current fork, but it's not effective + // yet. It indicates that the program has delayed visibility. Return + // the tombstone to reflect that. + Arc::new(LoadedProgram::new_tombstone( + entry.deployment_slot, + LoadedProgramType::DelayVisibility, + )) + } else { + continue; + }; + entry_to_return + .tx_usage_counter + .fetch_add(*usage_count, Ordering::Relaxed); + loaded.insert(*key, entry_to_return); + return false; } } - extracting.missing.insert(key, usage_count); - None - }) - .collect::>>(); - + } + true + }); drop(locked_fork_graph); self.stats .misses - .fetch_add(extracting.missing.len() as u64, Ordering::Relaxed); + .fetch_add(search_for.len() as u64, Ordering::Relaxed); self.stats .hits - .fetch_add(extracting.loaded.entries.len() as u64, Ordering::Relaxed); - drop(extracting); - extracted + .fetch_add(loaded.len() as u64, Ordering::Relaxed); + LoadedProgramsForTxBatch { + entries: loaded, + slot: current_slot, + environments: environments.clone(), + } } pub fn merge(&mut self, tx_batch_cache: &LoadedProgramsForTxBatch) { @@ -973,9 +960,9 @@ impl solana_frozen_abi::abi_example::AbiExample for LoadedProgram mod tests { use { crate::loaded_programs::{ - BlockRelation, ExtractedPrograms, ForkGraph, LoadedProgram, LoadedProgramMatchCriteria, - LoadedProgramType, LoadedPrograms, ProgramRuntimeEnvironment, - ProgramRuntimeEnvironments, DELAY_VISIBILITY_SLOT_OFFSET, + BlockRelation, ForkGraph, LoadedProgram, LoadedProgramMatchCriteria, LoadedProgramType, + LoadedPrograms, ProgramRuntimeEnvironment, ProgramRuntimeEnvironments, + DELAY_VISIBILITY_SLOT_OFFSET, }, assert_matches::assert_matches, percentage::Percentage, @@ -985,7 +972,7 @@ mod tests { ops::ControlFlow, sync::{ atomic::{AtomicU64, Ordering}, - Arc, Mutex, RwLock, + Arc, RwLock, }, }, }; @@ -1572,27 +1559,25 @@ mod tests { } fn match_slot( - extracted: &Arc>, + extracted: &LoadedProgramsForTxBatch, program: &Pubkey, deployment_slot: Slot, working_slot: Slot, ) -> bool { - let extracted = extracted.lock().unwrap(); - assert_eq!(extracted.loaded.slot, working_slot); + assert_eq!(extracted.slot, working_slot); extracted - .loaded - .find(program) + .entries + .get(program) .map(|entry| entry.deployment_slot == deployment_slot) .unwrap_or(false) } fn match_missing( - extracted: &Arc>, - key: &Pubkey, + missing: &[(Pubkey, (LoadedProgramMatchCriteria, u64))], + program: &Pubkey, _reload: bool, ) -> bool { - let extracted = extracted.lock().unwrap(); - extracted.missing.get(key).is_some() + missing.iter().any(|(key, _)| key == program) } #[test] @@ -1673,34 +1658,28 @@ mod tests { // 23 // Testing fork 0 - 10 - 12 - 22 with current slot at 22 - let extracted = cache.extract( - 22, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 2)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 3)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 4)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 2)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 3)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 4)), + ]; + let extracted = cache.extract(22, &mut missing); assert!(match_slot(&extracted, &program1, 20, 22)); assert!(match_slot(&extracted, &program4, 0, 22)); - assert!(match_missing(&extracted, &program2, false)); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program2, false)); + assert!(match_missing(&missing, &program3, false)); // Testing fork 0 - 5 - 11 - 15 - 16 with current slot at 16 - let extracted = cache.extract( - 15, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(15, &mut missing); assert!(match_slot(&extracted, &program1, 0, 15)); assert!(match_slot(&extracted, &program2, 11, 15)); @@ -1708,27 +1687,21 @@ mod tests { // The effective slot of program4 deployed in slot 15 is 19. So it should not be usable in slot 16. // A delay visibility tombstone should be returned here. let tombstone = extracted - .lock() - .unwrap() - .loaded .find(&program4) .expect("Failed to find the tombstone"); assert_matches!(tombstone.program, LoadedProgramType::DelayVisibility); assert_eq!(tombstone.deployment_slot, 15); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program3, false)); // Testing the same fork above, but current slot is now 18 (equal to effective slot of program4). - let extracted = cache.extract( - 18, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(18, &mut missing); assert!(match_slot(&extracted, &program1, 0, 18)); assert!(match_slot(&extracted, &program2, 11, 18)); @@ -1736,19 +1709,16 @@ mod tests { // The effective slot of program4 deployed in slot 15 is 18. So it should be usable in slot 18. assert!(match_slot(&extracted, &program4, 15, 18)); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program3, false)); // Testing the same fork above, but current slot is now 23 (future slot than effective slot of program4). - let extracted = cache.extract( - 23, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(23, &mut missing); assert!(match_slot(&extracted, &program1, 0, 23)); assert!(match_slot(&extracted, &program2, 11, 23)); @@ -1756,33 +1726,27 @@ mod tests { // The effective slot of program4 deployed in slot 15 is 19. So it should be usable in slot 23. assert!(match_slot(&extracted, &program4, 15, 23)); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program3, false)); // Testing fork 0 - 5 - 11 - 15 - 16 with current slot at 11 - let extracted = cache.extract( - 11, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(11, &mut missing); assert!(match_slot(&extracted, &program1, 0, 11)); // program2 was updated at slot 11, but is not effective till slot 12. The result should contain a tombstone. let tombstone = extracted - .lock() - .unwrap() - .loaded .find(&program2) .expect("Failed to find the tombstone"); assert_matches!(tombstone.program, LoadedProgramType::DelayVisibility); assert_eq!(tombstone.deployment_slot, 11); assert!(match_slot(&extracted, &program4, 5, 11)); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program3, false)); // The following is a special case, where there's an expiration slot let test_program = Arc::new(LoadedProgram { @@ -1797,42 +1761,36 @@ mod tests { assert!(!cache.replenish(program4, test_program).0); // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 - let extracted = cache.extract( - 19, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(19, &mut missing); 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(&extracted, &program3, false)); + 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 extracted = cache.extract( - 21, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(21, &mut missing); assert!(match_slot(&extracted, &program1, 0, 21)); assert!(match_slot(&extracted, &program2, 11, 21)); - assert!(match_missing(&extracted, &program3, false)); - assert!(match_missing(&extracted, &program4, false)); + 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(programs) = cache.entries.get_mut(&program4) { @@ -1857,35 +1815,29 @@ mod tests { // 23 // Testing fork 11 - 15 - 16- 19 - 22 with root at 5 and current slot at 22 - let extracted = cache.extract( - 21, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(21, &mut missing); // Since the fork was pruned, we should not find the entry deployed at slot 20. assert!(match_slot(&extracted, &program1, 0, 21)); assert!(match_slot(&extracted, &program2, 11, 21)); assert!(match_slot(&extracted, &program4, 15, 21)); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program3, false)); // Testing fork 0 - 5 - 11 - 25 - 27 with current slot at 27 - let extracted = cache.extract( - 27, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(27, &mut missing); assert!(match_slot(&extracted, &program1, 0, 27)); assert!(match_slot(&extracted, &program2, 11, 27)); @@ -1910,23 +1862,20 @@ mod tests { // 23 // Testing fork 16, 19, 23, with root at 15, current slot at 23 - let extracted = cache.extract( - 23, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(23, &mut missing); assert!(match_slot(&extracted, &program1, 0, 23)); assert!(match_slot(&extracted, &program2, 11, 23)); assert!(match_slot(&extracted, &program4, 15, 23)); // program3 was deployed on slot 25, which has been pruned - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program3, false)); } #[test] @@ -1968,42 +1917,36 @@ mod tests { assert!(!cache.replenish(program3, new_test_loaded_program(25, 26)).0); // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 - let extracted = cache.extract( - 12, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(12, &mut missing); assert!(match_slot(&extracted, &program1, 0, 12)); assert!(match_slot(&extracted, &program2, 11, 12)); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program3, false)); // Test the same fork, but request the program modified at a later slot than what's in the cache. - let extracted = cache.extract( - 12, - vec![ - ( - program1, - (LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(5), 1), - ), - ( - program2, - (LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(5), 1), - ), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + ( + program1, + (LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(5), 1), + ), + ( + program2, + (LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(5), 1), + ), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(12, &mut missing); assert!(match_slot(&extracted, &program2, 11, 12)); - assert!(match_missing(&extracted, &program1, false)); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program1, false)); + assert!(match_missing(&missing, &program3, false)); } #[test] @@ -2062,52 +2005,43 @@ mod tests { ); // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 - let extracted = cache.extract( - 19, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(19, &mut missing); assert!(match_slot(&extracted, &program1, 0, 19)); assert!(match_slot(&extracted, &program2, 11, 19)); - assert!(match_missing(&extracted, &program3, false)); + assert!(match_missing(&missing, &program3, false)); // Testing fork 0 - 5 - 11 - 25 - 27 with current slot at 27 - let extracted = cache.extract( - 27, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(27, &mut missing); assert!(match_slot(&extracted, &program1, 0, 27)); assert!(match_slot(&extracted, &program2, 11, 27)); - assert!(match_missing(&extracted, &program3, true)); + assert!(match_missing(&missing, &program3, true)); // Testing fork 0 - 10 - 20 - 22 with current slot at 22 - let extracted = cache.extract( - 22, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(22, &mut missing); assert!(match_slot(&extracted, &program1, 20, 22)); - assert!(match_missing(&extracted, &program2, false)); - assert!(match_missing(&extracted, &program3, true)); + assert!(match_missing(&missing, &program2, false)); + assert!(match_missing(&missing, &program3, true)); } #[test] @@ -2160,38 +2094,32 @@ mod tests { assert!(!cache.replenish(program1, test_program).0); // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 - let extracted = cache.extract( - 12, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(12, &mut missing); // 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(&extracted, &program3, false)); + 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 extracted = cache.extract( - 15, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(15, &mut missing); assert!(match_slot(&extracted, &program2, 11, 15)); - assert!(match_missing(&extracted, &program1, false)); - assert!(match_missing(&extracted, &program3, false)); + 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!( @@ -2245,19 +2173,13 @@ mod tests { cache.prune(10, 0); - let extracted = cache.extract( - 20, - vec![(program1, (LoadedProgramMatchCriteria::NoCriteria, 1))].into_iter(), - ); + let mut missing = vec![(program1, (LoadedProgramMatchCriteria::NoCriteria, 1))]; + let extracted = cache.extract(20, &mut missing); // The cache should have the program deployed at slot 0 assert_eq!( extracted - .lock() - .unwrap() - .loaded - .entries - .get(&program1) + .find(&program1) .expect("Did not find the program") .deployment_slot, 0 @@ -2291,73 +2213,58 @@ mod tests { let program2 = Pubkey::new_unique(); assert!(!cache.replenish(program2, new_test_loaded_program(10, 11)).0); - let extracted = cache.extract( - 20, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(20, &mut missing); assert!(match_slot(&extracted, &program1, 0, 20)); assert!(match_slot(&extracted, &program2, 10, 20)); - let extracted = cache.extract( - 6, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(6, &mut missing); assert!(match_slot(&extracted, &program1, 5, 6)); - assert!(match_missing(&extracted, &program2, false)); + assert!(match_missing(&missing, &program2, false)); // Pruning slot 5 will remove program1 entry deployed at slot 5. // On fork chaining from slot 5, the entry deployed at slot 0 will become visible. cache.prune_by_deployment_slot(5); - let extracted = cache.extract( - 20, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(20, &mut missing); assert!(match_slot(&extracted, &program1, 0, 20)); assert!(match_slot(&extracted, &program2, 10, 20)); - let extracted = cache.extract( - 6, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(6, &mut missing); assert!(match_slot(&extracted, &program1, 0, 6)); - assert!(match_missing(&extracted, &program2, false)); + assert!(match_missing(&missing, &program2, false)); // Pruning slot 10 will remove program2 entry deployed at slot 10. // As there is no other entry for program2, extract() will return it as missing. cache.prune_by_deployment_slot(10); - let extracted = cache.extract( - 20, - vec![ - (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), - (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), - ] - .into_iter(), - ); + let mut missing = vec![ + (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), + (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), + ]; + let extracted = cache.extract(20, &mut missing); assert!(match_slot(&extracted, &program1, 0, 20)); - assert!(match_missing(&extracted, &program2, false)); + assert!(match_missing(&missing, &program2, false)); } #[test] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index a552eff46051e9..e345bf60ca4c00 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -112,8 +112,8 @@ use { compute_budget_processor::process_compute_budget_instructions, invoke_context::BuiltinFunctionWithContext, loaded_programs::{ - ExtractedPrograms, LoadProgramMetrics, LoadedProgram, LoadedProgramMatchCriteria, - LoadedProgramType, LoadedPrograms, LoadedProgramsForTxBatch, ProgramRuntimeEnvironment, + LoadProgramMetrics, LoadedProgram, LoadedProgramMatchCriteria, LoadedProgramType, + LoadedPrograms, LoadedProgramsForTxBatch, ProgramRuntimeEnvironment, ProgramRuntimeEnvironments, DELAY_VISIBILITY_SLOT_OFFSET, }, log_collector::LogCollector, @@ -4985,7 +4985,7 @@ impl Bank { &self, program_accounts_map: &HashMap, ) -> LoadedProgramsForTxBatch { - let programs_and_slots: Vec<(Pubkey, (LoadedProgramMatchCriteria, u64))> = + let mut missing_programs: Vec<(Pubkey, (LoadedProgramMatchCriteria, u64))> = if self.check_program_modification_slot { program_accounts_map .iter() @@ -5011,27 +5011,20 @@ impl Bank { .collect() }; - let ExtractedPrograms { - loaded: mut loaded_programs_for_txs, - missing, - } = { + let mut loaded_programs_for_txs = { // Lock the global cache to figure out which programs need to be loaded let loaded_programs_cache = self.loaded_programs_cache.read().unwrap(); - Mutex::into_inner( - Arc::into_inner( - loaded_programs_cache.extract(self.slot, programs_and_slots.into_iter()), - ) - .unwrap(), - ) - .unwrap() + loaded_programs_cache.extract(self.slot, &mut missing_programs) }; // Load missing programs while global cache is unlocked - let missing_programs: Vec<(Pubkey, Arc)> = missing + let missing_programs: Vec<(Pubkey, Arc)> = missing_programs .iter() - .map(|(key, count)| { + .map(|(key, (_match_criteria, usage_count))| { let program = self.load_program(key, false, None); - program.tx_usage_counter.store(*count, Ordering::Relaxed); + program + .tx_usage_counter + .store(*usage_count, Ordering::Relaxed); (*key, program) }) .collect(); From 0284ee9f96a3bfcbd120143d7c3bba476b4e30b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 11 Dec 2023 18:02:51 +0000 Subject: [PATCH 3/9] Refactors LoadedPrograms::extract() to use a mutable parameter instead of returning a LoadedProgramsForTxBatch. --- program-runtime/src/loaded_programs.rs | 151 +++++++++++++++---------- runtime/src/bank.rs | 9 +- 2 files changed, 97 insertions(+), 63 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 50bad6260d7373..978892bd28f696 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -766,53 +766,61 @@ impl LoadedPrograms { /// and returns which program accounts the accounts DB needs to load. pub fn extract( &self, - current_slot: Slot, search_for: &mut Vec<(Pubkey, (LoadedProgramMatchCriteria, u64))>, - ) -> LoadedProgramsForTxBatch { + loaded_programs_for_tx_batch: &mut LoadedProgramsForTxBatch, + ) { debug_assert!(self.fork_graph.is_some()); let locked_fork_graph = self.fork_graph.as_ref().unwrap().read().unwrap(); - let current_epoch = locked_fork_graph.slot_epoch(current_slot).unwrap(); - let environments = self.get_environments_for_epoch(current_epoch); - let mut loaded = HashMap::new(); search_for.retain(|(key, (match_criteria, usage_count))| { if let Some(second_level) = self.entries.get(&key) { for entry in second_level.iter().rev() { let is_ancestor = matches!( - locked_fork_graph.relationship(entry.deployment_slot, current_slot), + locked_fork_graph + .relationship(entry.deployment_slot, loaded_programs_for_tx_batch.slot), BlockRelation::Ancestor ); if entry.deployment_slot <= self.latest_root_slot - || entry.deployment_slot == current_slot + || entry.deployment_slot == loaded_programs_for_tx_batch.slot || is_ancestor { - let entry_to_return = if current_slot >= entry.effective_slot { - if !Self::is_entry_usable(entry, current_slot, &match_criteria) - || !Self::matches_environment(entry, environments) - { - break; - } + let entry_to_return = + if loaded_programs_for_tx_batch.slot >= entry.effective_slot { + if !Self::is_entry_usable( + entry, + loaded_programs_for_tx_batch.slot, + match_criteria, + ) || !Self::matches_environment( + entry, + &loaded_programs_for_tx_batch.environments, + ) { + break; + } - if let LoadedProgramType::Unloaded(_environment) = &entry.program { - break; - } + if let LoadedProgramType::Unloaded(_environment) = &entry.program { + break; + } - entry.clone() - } else if entry.is_implicit_delay_visibility_tombstone(current_slot) { - // Found a program entry on the current fork, but it's not effective - // yet. It indicates that the program has delayed visibility. Return - // the tombstone to reflect that. - Arc::new(LoadedProgram::new_tombstone( - entry.deployment_slot, - LoadedProgramType::DelayVisibility, - )) - } else { - continue; - }; + entry.clone() + } else if entry.is_implicit_delay_visibility_tombstone( + loaded_programs_for_tx_batch.slot, + ) { + // Found a program entry on the current fork, but it's not effective + // yet. It indicates that the program has delayed visibility. Return + // the tombstone to reflect that. + Arc::new(LoadedProgram::new_tombstone( + entry.deployment_slot, + LoadedProgramType::DelayVisibility, + )) + } else { + continue; + }; entry_to_return .tx_usage_counter .fetch_add(*usage_count, Ordering::Relaxed); - loaded.insert(*key, entry_to_return); + loaded_programs_for_tx_batch + .entries + .insert(*key, entry_to_return); return false; } } @@ -823,14 +831,10 @@ impl LoadedPrograms { self.stats .misses .fetch_add(search_for.len() as u64, Ordering::Relaxed); - self.stats - .hits - .fetch_add(loaded.len() as u64, Ordering::Relaxed); - LoadedProgramsForTxBatch { - entries: loaded, - slot: current_slot, - environments: environments.clone(), - } + self.stats.hits.fetch_add( + loaded_programs_for_tx_batch.entries.len() as u64, + Ordering::Relaxed, + ); } pub fn merge(&mut self, tx_batch_cache: &LoadedProgramsForTxBatch) { @@ -961,8 +965,8 @@ mod tests { use { crate::loaded_programs::{ BlockRelation, ForkGraph, LoadedProgram, LoadedProgramMatchCriteria, LoadedProgramType, - LoadedPrograms, ProgramRuntimeEnvironment, ProgramRuntimeEnvironments, - DELAY_VISIBILITY_SLOT_OFFSET, + LoadedPrograms, LoadedProgramsForTxBatch, ProgramRuntimeEnvironment, + ProgramRuntimeEnvironments, DELAY_VISIBILITY_SLOT_OFFSET, }, assert_matches::assert_matches, percentage::Percentage, @@ -1664,7 +1668,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 3)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 4)), ]; - let extracted = cache.extract(22, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(22, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 20, 22)); assert!(match_slot(&extracted, &program4, 0, 22)); @@ -1679,7 +1684,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(15, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(15, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 15)); assert!(match_slot(&extracted, &program2, 11, 15)); @@ -1701,7 +1707,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(18, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(18, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 18)); assert!(match_slot(&extracted, &program2, 11, 18)); @@ -1718,7 +1725,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(23, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(23, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 23)); assert!(match_slot(&extracted, &program2, 11, 23)); @@ -1735,7 +1743,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(11, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(11, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 11)); // program2 was updated at slot 11, but is not effective till slot 12. The result should contain a tombstone. @@ -1767,7 +1776,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(19, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(19, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 19)); assert!(match_slot(&extracted, &program2, 11, 19)); @@ -1784,7 +1794,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(21, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(21, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 21)); assert!(match_slot(&extracted, &program2, 11, 21)); @@ -1821,7 +1832,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(21, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(21, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); // Since the fork was pruned, we should not find the entry deployed at slot 20. assert!(match_slot(&extracted, &program1, 0, 21)); @@ -1837,7 +1849,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(27, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(27, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 27)); assert!(match_slot(&extracted, &program2, 11, 27)); @@ -1868,7 +1881,8 @@ mod tests { (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program4, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(23, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(23, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 23)); assert!(match_slot(&extracted, &program2, 11, 23)); @@ -1922,7 +1936,8 @@ mod tests { (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(12, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(12, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 12)); assert!(match_slot(&extracted, &program2, 11, 12)); @@ -1941,7 +1956,8 @@ mod tests { ), (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(12, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(12, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program2, 11, 12)); @@ -2010,7 +2026,8 @@ mod tests { (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(19, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(19, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 19)); assert!(match_slot(&extracted, &program2, 11, 19)); @@ -2023,7 +2040,8 @@ mod tests { (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(27, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(27, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 27)); assert!(match_slot(&extracted, &program2, 11, 27)); @@ -2036,7 +2054,8 @@ mod tests { (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(22, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(22, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 20, 22)); @@ -2099,7 +2118,8 @@ mod tests { (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(12, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(12, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); // Program1 deployed at slot 11 should not be expired yet assert!(match_slot(&extracted, &program1, 11, 12)); @@ -2114,7 +2134,8 @@ mod tests { (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program3, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(15, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(15, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program2, 11, 15)); @@ -2174,7 +2195,8 @@ mod tests { cache.prune(10, 0); let mut missing = vec![(program1, (LoadedProgramMatchCriteria::NoCriteria, 1))]; - let extracted = cache.extract(20, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); // The cache should have the program deployed at slot 0 assert_eq!( @@ -2217,7 +2239,8 @@ mod tests { (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(20, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 20)); assert!(match_slot(&extracted, &program2, 10, 20)); @@ -2226,7 +2249,8 @@ mod tests { (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(6, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(6, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 5, 6)); assert!(match_missing(&missing, &program2, false)); @@ -2239,7 +2263,8 @@ mod tests { (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(20, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 20)); assert!(match_slot(&extracted, &program2, 10, 20)); @@ -2248,7 +2273,8 @@ mod tests { (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(6, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(6, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 6)); assert!(match_missing(&missing, &program2, false)); @@ -2261,7 +2287,8 @@ mod tests { (program1, (LoadedProgramMatchCriteria::NoCriteria, 1)), (program2, (LoadedProgramMatchCriteria::NoCriteria, 1)), ]; - let extracted = cache.extract(20, &mut missing); + let mut extracted = LoadedProgramsForTxBatch::new(20, cache.environments.clone()); + cache.extract(&mut missing, &mut extracted); assert!(match_slot(&extracted, &program1, 0, 20)); assert!(match_missing(&missing, &program2, false)); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e345bf60ca4c00..3924b7708b4177 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5014,7 +5014,14 @@ impl Bank { let mut loaded_programs_for_txs = { // Lock the global cache to figure out which programs need to be loaded let loaded_programs_cache = self.loaded_programs_cache.read().unwrap(); - loaded_programs_cache.extract(self.slot, &mut missing_programs) + let mut loaded_programs_for_txs = LoadedProgramsForTxBatch::new( + self.slot, + loaded_programs_cache + .get_environments_for_epoch(self.epoch) + .clone(), + ); + loaded_programs_cache.extract(&mut missing_programs, &mut loaded_programs_for_txs); + loaded_programs_for_txs }; // Load missing programs while global cache is unlocked From a1f0b01714f108feb62a476dc16c2b3c78aeb673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 11 Dec 2023 16:21:08 +0000 Subject: [PATCH 4/9] Adds explicit SecondLevel structure to LoadedPrograms. --- program-runtime/src/loaded_programs.rs | 102 +++++++++++++++---------- 1 file changed, 63 insertions(+), 39 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 978892bd28f696..35c78fa2d0fbd7 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -439,11 +439,16 @@ impl Default for ProgramRuntimeEnvironments { } } +#[derive(Debug, Default)] +struct SecondLevel { + slot_versions: Vec>, +} + pub struct LoadedPrograms { /// A two level index: /// - /// Pubkey is the address of a program, multiple versions can coexists simultaneously under the same address (in different slots). - entries: HashMap>>, + /// The first level is for the address at which programs are deployed and the second level for the slot (and thus also fork). + entries: HashMap, /// The slot of the last rerooting pub latest_root_slot: Slot, /// The epoch of the last rerooting @@ -577,12 +582,12 @@ impl LoadedPrograms { key: Pubkey, entry: Arc, ) -> (bool, Arc) { - let second_level = self.entries.entry(key).or_default(); - let index = second_level + let slot_versions = &mut self.entries.entry(key).or_default().slot_versions; + let index = slot_versions .iter() .position(|at| at.effective_slot >= entry.effective_slot); if let Some((existing, entry_index)) = - index.and_then(|index| second_level.get(index).map(|value| (value, index))) + index.and_then(|index| slot_versions.get(index).map(|value| (value, index))) { if existing.deployment_slot == entry.deployment_slot && existing.effective_slot == entry.effective_slot @@ -598,13 +603,13 @@ impl LoadedPrograms { existing.ix_usage_counter.load(Ordering::Relaxed), Ordering::Relaxed, ); - second_level.remove(entry_index); + slot_versions.remove(entry_index); } else if existing.is_tombstone() != entry.is_tombstone() { // Either the old entry is tombstone and the new one is not. // (Let's give the new entry a chance). // Or, the old entry is not a tombstone and the new one is a tombstone. // (Remove the old entry, as the tombstone makes it obsolete). - second_level.remove(entry_index); + slot_versions.remove(entry_index); } else { self.stats.replacements.fetch_add(1, Ordering::Relaxed); return (true, existing.clone()); @@ -612,7 +617,7 @@ impl LoadedPrograms { } } self.stats.insertions.fetch_add(1, Ordering::Relaxed); - second_level.insert(index.unwrap_or(second_level.len()), entry.clone()); + slot_versions.insert(index.unwrap_or(slot_versions.len()), entry.clone()); (false, entry) } @@ -628,7 +633,9 @@ impl LoadedPrograms { pub fn prune_by_deployment_slot(&mut self, slot: Slot) { for second_level in self.entries.values_mut() { - second_level.retain(|entry| entry.deployment_slot != slot); + second_level + .slot_versions + .retain(|entry| entry.deployment_slot != slot); } self.remove_programs_with_no_entries(); } @@ -656,7 +663,8 @@ impl LoadedPrograms { // Remove entries un/re/deployed on orphan forks let mut first_ancestor_found = false; let mut first_ancestor_env = None; - *second_level = second_level + second_level.slot_versions = second_level + .slot_versions .iter() .rev() .filter(|entry| { @@ -712,7 +720,7 @@ impl LoadedPrograms { }) .cloned() .collect(); - second_level.reverse(); + second_level.slot_versions.reverse(); } self.remove_programs_with_no_entries(); debug_assert!(self.latest_root_slot <= new_root_slot); @@ -773,7 +781,7 @@ impl LoadedPrograms { let locked_fork_graph = self.fork_graph.as_ref().unwrap().read().unwrap(); search_for.retain(|(key, (match_criteria, usage_count))| { if let Some(second_level) = self.entries.get(&key) { - for entry in second_level.iter().rev() { + for entry in second_level.slot_versions.iter().rev() { let is_ancestor = matches!( locked_fork_graph .relationship(entry.deployment_slot, loaded_programs_for_tx_batch.slot), @@ -853,8 +861,10 @@ impl LoadedPrograms { ) -> Vec<(Pubkey, Arc)> { self.entries .iter() - .flat_map(|(id, list)| { - list.iter() + .flat_map(|(id, second_level)| { + second_level + .slot_versions + .iter() .filter_map(move |program| match program.program { LoadedProgramType::LegacyV0(_) | LoadedProgramType::LegacyV1(_) if include_program_runtime_v1 => @@ -890,8 +900,8 @@ impl LoadedPrograms { } fn unload_program(&mut self, id: &Pubkey) { - if let Some(entries) = self.entries.get_mut(id) { - for entry in entries.iter_mut() { + if let Some(second_level) = self.entries.get_mut(id) { + for entry in second_level.slot_versions.iter_mut() { if let Some(unloaded) = entry.to_unloaded() { *entry = Arc::new(unloaded); self.stats @@ -914,8 +924,12 @@ impl LoadedPrograms { remove: impl Iterator)>, ) { for (id, program) in remove { - if let Some(entries) = self.entries.get_mut(id) { - if let Some(candidate) = entries.iter_mut().find(|entry| entry == &program) { + if let Some(second_level) = self.entries.get_mut(id) { + if let Some(candidate) = second_level + .slot_versions + .iter_mut() + .find(|entry| entry == &program) + { if let Some(unloaded) = candidate.to_unloaded() { if candidate.tx_usage_counter.load(Ordering::Relaxed) == 1 { self.stats.one_hit_wonders.fetch_add(1, Ordering::Relaxed); @@ -934,7 +948,8 @@ impl LoadedPrograms { fn remove_programs_with_no_entries(&mut self) { let num_programs_before_removal = self.entries.len(); - self.entries.retain(|_, programs| !programs.is_empty()); + self.entries + .retain(|_, second_level| !second_level.slot_versions.is_empty()); if self.entries.len() < num_programs_before_removal { self.stats.empty_entries.fetch_add( num_programs_before_removal.saturating_sub(self.entries.len()) as u64, @@ -1079,8 +1094,9 @@ mod tests { cache .entries .values() - .map(|programs| { - programs + .map(|second_level| { + second_level + .slot_versions .iter() .filter(|program| predicate(&program.program)) .count() @@ -1220,8 +1236,8 @@ mod tests { let unloaded = cache .entries .iter() - .flat_map(|(id, cached_programs)| { - cached_programs.iter().filter_map(|program| { + .flat_map(|(id, second_level)| { + second_level.slot_versions.iter().filter_map(|program| { matches!(program.program, LoadedProgramType::Unloaded(_)) .then_some((*id, program.tx_usage_counter.load(Ordering::Relaxed))) }) @@ -1274,8 +1290,8 @@ mod tests { }); assert_eq!(num_unloaded, 1); - cache.entries.values().for_each(|programs| { - programs.iter().for_each(|program| { + cache.entries.values().for_each(|second_level| { + second_level.slot_versions.iter().for_each(|program| { if matches!(program.program, LoadedProgramType::Unloaded(_)) { // Test that the usage counter is retained for the unloaded program assert_eq!(program.tx_usage_counter.load(Ordering::Relaxed), 10); @@ -1292,8 +1308,8 @@ mod tests { new_test_loaded_program_with_usage(0, 2, AtomicU64::new(0)), ); - cache.entries.values().for_each(|programs| { - programs.iter().for_each(|program| { + cache.entries.values().for_each(|second_level| { + second_level.slot_versions.iter().for_each(|program| { if matches!(program.program, LoadedProgramType::Unloaded(_)) && program.deployment_slot == 0 && program.effective_slot == 2 @@ -1351,8 +1367,8 @@ mod tests { .entries .get(&program1) .expect("Failed to find the entry"); - assert_eq!(second_level.len(), 1); - assert!(second_level.first().unwrap().is_tombstone()); + assert_eq!(second_level.slot_versions.len(), 1); + assert!(second_level.slot_versions.first().unwrap().is_tombstone()); assert_eq!(tombstone.deployment_slot, 10); assert_eq!(tombstone.effective_slot, 10); @@ -1367,8 +1383,8 @@ mod tests { .entries .get(&program2) .expect("Failed to find the entry"); - assert_eq!(second_level.len(), 1); - assert!(!second_level.first().unwrap().is_tombstone()); + assert_eq!(second_level.slot_versions.len(), 1); + assert!(!second_level.slot_versions.first().unwrap().is_tombstone()); let tombstone = set_tombstone( &mut cache, @@ -1380,9 +1396,9 @@ mod tests { .entries .get(&program2) .expect("Failed to find the entry"); - assert_eq!(second_level.len(), 2); - assert!(!second_level.first().unwrap().is_tombstone()); - assert!(second_level.get(1).unwrap().is_tombstone()); + assert_eq!(second_level.slot_versions.len(), 2); + assert!(!second_level.slot_versions.first().unwrap().is_tombstone()); + assert!(second_level.slot_versions.get(1).unwrap().is_tombstone()); assert!(tombstone.is_tombstone()); assert_eq!(tombstone.deployment_slot, 60); assert_eq!(tombstone.effective_slot, 60); @@ -1491,6 +1507,7 @@ mod tests { .entries .get(&program1) .expect("failed to find the program") + .slot_versions .len(), 2 ); @@ -1503,20 +1520,25 @@ mod tests { .entries .get(&program1) .expect("failed to find the program") + .slot_versions .len(), 2 ); cache.prune(22, cache.latest_root_epoch.saturating_add(1)); - let entries = cache + let second_level = cache .entries .get(&program1) .expect("failed to find the program"); // Test that prune removed 1 entry, since epoch changed - assert_eq!(entries.len(), 1); + assert_eq!(second_level.slot_versions.len(), 1); - let entry = entries.first().expect("Failed to get the program").clone(); + let entry = second_level + .slot_versions + .first() + .expect("Failed to get the program") + .clone(); // Test that the correct entry remains in the cache assert_eq!(entry, updated_program); } @@ -1804,8 +1826,8 @@ mod tests { assert!(match_missing(&missing, &program4, false)); // Remove the expired entry to let the rest of the test continue - if let Some(programs) = cache.entries.get_mut(&program4) { - programs.pop(); + if let Some(second_level) = cache.entries.get_mut(&program4) { + second_level.slot_versions.pop(); } cache.prune(5, 0); @@ -2148,6 +2170,7 @@ mod tests { .entries .get(&program1) .expect("Didn't find program1") + .slot_versions .len(), 3 ); @@ -2159,6 +2182,7 @@ mod tests { .entries .get(&program1) .expect("Didn't find program1") + .slot_versions .len(), 1 ); From 54c9e7d5519494ee7cc3043db217ba6e988d2494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 11 Dec 2023 18:24:50 +0000 Subject: [PATCH 5/9] Adds cooperative_loading_task. --- program-runtime/src/loaded_programs.rs | 47 +++++++++++++++++++++++--- runtime/src/bank.rs | 5 +-- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 35c78fa2d0fbd7..ac44167ad7c80c 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -442,6 +442,8 @@ impl Default for ProgramRuntimeEnvironments { #[derive(Debug, Default)] struct SecondLevel { slot_versions: Vec>, + /// Contains the bank and TX batch a program at this address is currently being loaded + cooperative_loading_lock: Option<(Slot, std::thread::ThreadId)>, } pub struct LoadedPrograms { @@ -773,14 +775,15 @@ impl LoadedPrograms { /// 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( - &self, + &mut self, search_for: &mut Vec<(Pubkey, (LoadedProgramMatchCriteria, u64))>, loaded_programs_for_tx_batch: &mut LoadedProgramsForTxBatch, - ) { + ) -> Option<(Pubkey, u64)> { debug_assert!(self.fork_graph.is_some()); let locked_fork_graph = self.fork_graph.as_ref().unwrap().read().unwrap(); + let mut cooperative_loading_task = None; search_for.retain(|(key, (match_criteria, usage_count))| { - if let Some(second_level) = self.entries.get(&key) { + if let Some(second_level) = self.entries.get_mut(key) { for entry in second_level.slot_versions.iter().rev() { let is_ancestor = matches!( locked_fork_graph @@ -833,6 +836,18 @@ impl LoadedPrograms { } } } + if cooperative_loading_task.is_none() { + // We have not selected a task so far + let second_level = self.entries.entry(*key).or_default(); + if second_level.cooperative_loading_lock.is_none() { + // Select this missing entry which is not selected by any other TX batch yet + cooperative_loading_task = Some((*key, *usage_count)); + second_level.cooperative_loading_lock = Some(( + loaded_programs_for_tx_batch.slot, + std::thread::current().id(), + )); + } + } true }); drop(locked_fork_graph); @@ -843,6 +858,23 @@ impl LoadedPrograms { loaded_programs_for_tx_batch.entries.len() as u64, Ordering::Relaxed, ); + cooperative_loading_task + } + + /// Called by Bank::replenish_program_cache() for each program that is done loading. + pub fn finish_cooperative_loading_task( + &mut self, + slot: Slot, + key: Pubkey, + loaded_program: Arc, + ) { + let second_level = self.entries.entry(key).or_default(); + debug_assert_eq!( + second_level.cooperative_loading_lock, + Some((slot, std::thread::current().id())) + ); + second_level.cooperative_loading_lock = None; + self.assign_program(key, loaded_program); } pub fn merge(&mut self, tx_batch_cache: &LoadedProgramsForTxBatch) { @@ -948,8 +980,10 @@ impl LoadedPrograms { fn remove_programs_with_no_entries(&mut self) { let num_programs_before_removal = self.entries.len(); - self.entries - .retain(|_, second_level| !second_level.slot_versions.is_empty()); + self.entries.retain(|_, second_level| { + !second_level.slot_versions.is_empty() + || second_level.cooperative_loading_lock.is_some() + }); if self.entries.len() < num_programs_before_removal { self.stats.empty_entries.fetch_add( num_programs_before_removal.saturating_sub(self.entries.len()) as u64, @@ -2187,6 +2221,9 @@ mod tests { 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()); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 3924b7708b4177..6774b44ba72e3d 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5013,14 +5013,15 @@ impl Bank { let mut loaded_programs_for_txs = { // Lock the global cache to figure out which programs need to be loaded - let loaded_programs_cache = self.loaded_programs_cache.read().unwrap(); + let mut loaded_programs_cache = self.loaded_programs_cache.write().unwrap(); let mut loaded_programs_for_txs = LoadedProgramsForTxBatch::new( self.slot, loaded_programs_cache .get_environments_for_epoch(self.epoch) .clone(), ); - loaded_programs_cache.extract(&mut missing_programs, &mut loaded_programs_for_txs); + let _cooperative_loading_task = + loaded_programs_cache.extract(&mut missing_programs, &mut loaded_programs_for_txs); loaded_programs_for_txs }; From 27ad693fd1c1e9caedfce8b52604f66fd34a3594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 11 Dec 2023 20:12:12 +0000 Subject: [PATCH 6/9] Implements cooperative loading in the bank. --- runtime/src/bank.rs | 74 +++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 6774b44ba72e3d..9e1bbd7b10d1be 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5011,40 +5011,50 @@ impl Bank { .collect() }; - let mut loaded_programs_for_txs = { - // Lock the global cache to figure out which programs need to be loaded - let mut loaded_programs_cache = self.loaded_programs_cache.write().unwrap(); - let mut loaded_programs_for_txs = LoadedProgramsForTxBatch::new( - self.slot, - loaded_programs_cache - .get_environments_for_epoch(self.epoch) - .clone(), - ); - let _cooperative_loading_task = - loaded_programs_cache.extract(&mut missing_programs, &mut loaded_programs_for_txs); - loaded_programs_for_txs - }; - - // Load missing programs while global cache is unlocked - let missing_programs: Vec<(Pubkey, Arc)> = missing_programs - .iter() - .map(|(key, (_match_criteria, usage_count))| { - let program = self.load_program(key, false, None); - program - .tx_usage_counter - .store(*usage_count, Ordering::Relaxed); - (*key, program) - }) - .collect(); + let mut loaded_programs_for_txs = None; + let mut program_to_store = None; + loop { + let program_to_load = { + // Lock the global cache. + let mut loaded_programs_cache = self.loaded_programs_cache.write().unwrap(); + // Initialize our local cache. + if loaded_programs_for_txs.is_none() { + loaded_programs_for_txs = Some(LoadedProgramsForTxBatch::new( + self.slot, + loaded_programs_cache + .get_environments_for_epoch(self.epoch) + .clone(), + )); + } + // Submit our last completed loading task. + if let Some((key, program)) = program_to_store.take() { + loaded_programs_cache.finish_cooperative_loading_task( + self.slot(), + key, + program, + ); + } + // Figure out which program needs to be loaded next. + loaded_programs_cache.extract( + &mut missing_programs, + loaded_programs_for_txs.as_mut().unwrap(), + ) + // Unlock the global cache again. + }; - // Lock the global cache again to replenish the missing programs - let mut loaded_programs_cache = self.loaded_programs_cache.write().unwrap(); - for (key, program) in missing_programs { - let (_was_occupied, entry) = loaded_programs_cache.replenish(key, program); - // Use the returned entry as that might have been deduplicated globally - loaded_programs_for_txs.replenish(key, entry); + if let Some((key, count)) = program_to_load { + // Load, verify and compile one program. + let program = self.load_program(&key, false, None); + program.tx_usage_counter.store(count, Ordering::Relaxed); + program_to_store = Some((key, program)); + } else if missing_programs.is_empty() { + break; + } else { + // TODO: Wait on a std::sync::Condvar + } } - loaded_programs_for_txs + + loaded_programs_for_txs.unwrap() } /// Returns a hash map of executable program accounts (program accounts that are not writable From 1d3b321f7186c9081f93d34adda1dfeebfd9a75b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 11 Dec 2023 20:12:52 +0000 Subject: [PATCH 7/9] Fixes fork graph in tests. --- programs/sbf/tests/programs.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 12b8f160411d27..3a33762baf6e3f 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -2111,10 +2111,13 @@ fn test_program_sbf_invoke_in_same_tx_as_redeployment() { ); // load_upgradeable_program sets clock sysvar to 1, which causes the program to be effective - // after 2 slots. So we need to advance the bank client by 2 slots here. + // after 2 slots. They need to be called individually to create the correct fork graph in between. + bank_client + .advance_slot(1, bank_forks.as_ref(), &Pubkey::default()) + .unwrap(); let bank = bank_client - .advance_slot(2, bank_forks.as_ref(), &Pubkey::default()) - .expect("Failed to advance slot"); + .advance_slot(1, bank_forks.as_ref(), &Pubkey::default()) + .unwrap(); // Prepare redeployment let buffer_keypair = Keypair::new(); @@ -2207,10 +2210,13 @@ fn test_program_sbf_invoke_in_same_tx_as_undeployment() { ); // load_upgradeable_program sets clock sysvar to 1, which causes the program to be effective - // after 2 slots. So we need to advance the bank client by 2 slots here. + // after 2 slots. They need to be called individually to create the correct fork graph in between. + bank_client + .advance_slot(1, bank_forks.as_ref(), &Pubkey::default()) + .unwrap(); let bank = bank_client - .advance_slot(2, bank_forks.as_ref(), &Pubkey::default()) - .expect("Failed to advance slot"); + .advance_slot(1, bank_forks.as_ref(), &Pubkey::default()) + .unwrap(); // Prepare undeployment let (programdata_address, _) = Pubkey::find_program_address( From e15ba317206d67f8859694205bcc987b7a57a982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 11 Dec 2023 20:35:31 +0000 Subject: [PATCH 8/9] Adds LoadingTaskWaiter. --- program-runtime/src/loaded_programs.rs | 53 +++++++++++++++++++++++++- runtime/src/bank.rs | 13 +++++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index ac44167ad7c80c..1a79353c530d1e 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -25,7 +25,7 @@ use { fmt::{Debug, Formatter}, sync::{ atomic::{AtomicU64, Ordering}, - Arc, RwLock, + Arc, Condvar, Mutex, RwLock, }, }, }; @@ -439,6 +439,54 @@ impl Default for ProgramRuntimeEnvironments { } } +#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)] +pub struct LoadingTaskCookie(u64); + +impl LoadingTaskCookie { + fn new() -> Self { + Self(0) + } + + fn update(&mut self) { + let LoadingTaskCookie(cookie) = self; + *cookie = cookie.wrapping_add(1); + } +} + +/// Prevents excessive polling during cooperative loading +#[derive(Debug, Default)] +pub struct LoadingTaskWaiter { + cookie: Mutex, + cond: Condvar, +} + +impl LoadingTaskWaiter { + pub fn new() -> Self { + Self { + cookie: Mutex::new(LoadingTaskCookie::new()), + cond: Condvar::new(), + } + } + + pub fn cookie(&self) -> LoadingTaskCookie { + *self.cookie.lock().unwrap() + } + + pub fn notify(&self) { + let mut cookie = self.cookie.lock().unwrap(); + cookie.update(); + self.cond.notify_all(); + } + + pub fn wait(&self, cookie: LoadingTaskCookie) -> LoadingTaskCookie { + let cookie_guard = self.cookie.lock().unwrap(); + *self + .cond + .wait_while(cookie_guard, |current_cookie| *current_cookie == cookie) + .unwrap() + } +} + #[derive(Debug, Default)] struct SecondLevel { slot_versions: Vec>, @@ -467,6 +515,7 @@ pub struct LoadedPrograms { pub programs_to_recompile: Vec<(Pubkey, Arc)>, pub stats: Stats, pub fork_graph: Option>>, + pub loading_task_waiter: Arc, } impl Debug for LoadedPrograms { @@ -559,6 +608,7 @@ impl LoadedPrograms { programs_to_recompile: Vec::default(), stats: Stats::default(), fork_graph: None, + loading_task_waiter: Arc::new(LoadingTaskWaiter::default()), } } @@ -875,6 +925,7 @@ impl LoadedPrograms { ); second_level.cooperative_loading_lock = None; self.assign_program(key, loaded_program); + self.loading_task_waiter.notify(); } pub fn merge(&mut self, tx_batch_cache: &LoadedProgramsForTxBatch) { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 9e1bbd7b10d1be..4d26e93bb67e15 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5014,7 +5014,7 @@ impl Bank { let mut loaded_programs_for_txs = None; let mut program_to_store = None; loop { - let program_to_load = { + let (program_to_load, task_cookie, task_waiter) = { // Lock the global cache. let mut loaded_programs_cache = self.loaded_programs_cache.write().unwrap(); // Initialize our local cache. @@ -5035,10 +5035,12 @@ impl Bank { ); } // Figure out which program needs to be loaded next. - loaded_programs_cache.extract( + let program_to_load = loaded_programs_cache.extract( &mut missing_programs, loaded_programs_for_txs.as_mut().unwrap(), - ) + ); + let task_waiter = Arc::clone(&loaded_programs_cache.loading_task_waiter); + (program_to_load, task_waiter.cookie(), task_waiter) // Unlock the global cache again. }; @@ -5050,7 +5052,10 @@ impl Bank { } else if missing_programs.is_empty() { break; } else { - // TODO: Wait on a std::sync::Condvar + // Sleep until the next finish_cooperative_loading_task() call. + // Once a task completes we'll wake up and try to load the + // missing programs inside the tx batch again. + let _new_cookie = task_waiter.wait(task_cookie); } } From 65860750ab34e9290b383317e39a07722824a810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Tue, 12 Dec 2023 20:27:32 +0000 Subject: [PATCH 9/9] Environment mismatch needs to just skip the entry. --- program-runtime/src/loaded_programs.rs | 59 +++++++++++++------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 1a79353c530d1e..1bbdadbf7ced62 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -845,37 +845,38 @@ impl LoadedPrograms { || entry.deployment_slot == loaded_programs_for_tx_batch.slot || is_ancestor { - let entry_to_return = - if loaded_programs_for_tx_batch.slot >= entry.effective_slot { - if !Self::is_entry_usable( - entry, - loaded_programs_for_tx_batch.slot, - match_criteria, - ) || !Self::matches_environment( - entry, - &loaded_programs_for_tx_batch.environments, - ) { - break; - } - - if let LoadedProgramType::Unloaded(_environment) = &entry.program { - break; - } - - entry.clone() - } else if entry.is_implicit_delay_visibility_tombstone( + let entry_to_return = if loaded_programs_for_tx_batch.slot + >= entry.effective_slot + && Self::matches_environment( + entry, + &loaded_programs_for_tx_batch.environments, + ) { + if !Self::is_entry_usable( + entry, loaded_programs_for_tx_batch.slot, + match_criteria, ) { - // Found a program entry on the current fork, but it's not effective - // yet. It indicates that the program has delayed visibility. Return - // the tombstone to reflect that. - Arc::new(LoadedProgram::new_tombstone( - entry.deployment_slot, - LoadedProgramType::DelayVisibility, - )) - } else { - continue; - }; + break; + } + + if let LoadedProgramType::Unloaded(_environment) = &entry.program { + break; + } + + entry.clone() + } else if entry.is_implicit_delay_visibility_tombstone( + loaded_programs_for_tx_batch.slot, + ) { + // Found a program entry on the current fork, but it's not effective + // yet. It indicates that the program has delayed visibility. Return + // the tombstone to reflect that. + Arc::new(LoadedProgram::new_tombstone( + entry.deployment_slot, + LoadedProgramType::DelayVisibility, + )) + } else { + continue; + }; entry_to_return .tx_usage_counter .fetch_add(*usage_count, Ordering::Relaxed);