From e21856cda6e8c200f90c224b03842e1b3f2666e3 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Fri, 8 Dec 2023 13:05:22 -0800 Subject: [PATCH 1/8] Use 2's random selection to evict program cache --- program-runtime/src/loaded_programs.rs | 102 ++++++++++++++++++++----- runtime/src/bank.rs | 5 +- 2 files changed, 86 insertions(+), 21 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 1bbdadbf7ced62..385d68ffbdc7a8 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -6,6 +6,7 @@ use { itertools::Itertools, log::{debug, error, log_enabled, trace}, percentage::PercentageInteger, + rand::{thread_rng, Rng}, solana_measure::measure::Measure, solana_rbpf::{ elf::Executable, @@ -416,6 +417,10 @@ impl LoadedProgram { && slot >= self.deployment_slot && slot < self.effective_slot } + + fn decayed_usage_counter(&self, _now: Slot) -> u64 { + self.tx_usage_counter.load(Ordering::Relaxed) + } } #[derive(Clone, Debug)] @@ -935,6 +940,26 @@ impl LoadedPrograms { }) } + /// Returns the list of loaded programs which are verified and compiled. + fn get_flattened_entries(&self) -> Vec<(Pubkey, Arc)> { + self.entries + .iter() + .flat_map(|(id, second_level)| { + second_level + .slot_versions + .iter() + .filter_map(move |program| match program.program { + LoadedProgramType::LegacyV0(_) + | LoadedProgramType::LegacyV1(_) + | LoadedProgramType::Typed(_) => Some((*id, program.clone())), + #[cfg(test)] + LoadedProgramType::TestLoaded(_) => Some((*id, program.clone())), + _ => None, + }) + }) + .collect() + } + /// Returns the list of loaded programs which are verified and compiled sorted by `tx_usage_counter`. /// /// Entries from program runtime v1 and v2 can be individually filtered. @@ -976,6 +1001,39 @@ impl LoadedPrograms { self.unload_program_entries(sorted_candidates.iter().take(num_to_unload)); } + /// Evicts programs using 2's random selection, choosing the least used program out of the two entries. + /// The eviction is performed enough number of times to reduce the cache usage to the given percentage. + pub fn evict_using_2s_random_selection(&mut self, shrink_to: PercentageInteger, now: Slot) { + let mut candidates = self.get_flattened_entries(); + let num_to_unload = candidates + .len() + .saturating_sub(shrink_to.apply_to(MAX_LOADED_ENTRY_COUNT)); + for _ in 0..num_to_unload { + let mut rng = thread_rng(); + let index1 = rng.gen_range(0..candidates.len()); + let index2 = rng.gen_range(0..candidates.len()); + if let Some((usage_counter1, usage_counter2)) = + candidates.get(index1).and_then(|(_id, entry1)| { + candidates.get(index2).map(|(_id, entry2)| { + ( + entry1.decayed_usage_counter(now), + entry2.decayed_usage_counter(now), + ) + }) + }) + { + let (program, entry) = if usage_counter1 < usage_counter2 { + candidates.remove(index1) + } else { + candidates.remove(index2) + }; + self.unload_program_entry(&program, &entry); + } else { + error!("Failed in evicting an entry from the program cache"); + } + } + } + /// Removes all the entries at the given keys, if they exist pub fn remove_programs(&mut self, keys: impl Iterator) { for k in keys { @@ -1003,30 +1061,34 @@ impl LoadedPrograms { keys.iter().for_each(|key| self.unload_program(key)); } + fn unload_program_entry(&mut self, program: &Pubkey, remove_entry: &Arc) { + if let Some(second_level) = self.entries.get_mut(program) { + if let Some(candidate) = second_level + .slot_versions + .iter_mut() + .find(|entry| entry == &remove_entry) + { + 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); + } + self.stats + .evictions + .entry(*program) + .and_modify(|c| saturating_add_assign!(*c, 1)) + .or_insert(1); + *candidate = Arc::new(unloaded); + } + } + } + } + fn unload_program_entries<'a>( &mut self, remove: impl Iterator)>, ) { - for (id, program) in remove { - 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); - } - self.stats - .evictions - .entry(*id) - .and_modify(|c| saturating_add_assign!(*c, 1)) - .or_insert(1); - *candidate = Arc::new(unloaded); - } - } - } + for (program, entry) in remove { + self.unload_program_entry(program, entry); } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 0ee073d32ae01f..f8c87e11312ea2 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5287,7 +5287,10 @@ impl Bank { self.loaded_programs_cache .write() .unwrap() - .sort_and_unload(Percentage::from(SHRINK_LOADED_PROGRAMS_TO_PERCENTAGE)); + .evict_using_2s_random_selection( + Percentage::from(SHRINK_LOADED_PROGRAMS_TO_PERCENTAGE), + self.slot(), + ); debug!( "check: {}us load: {}us execute: {}us txs_len={}", From 03f0179880beb751b860e36a966a87d8ddfdf18d Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Sun, 10 Dec 2023 08:32:47 -0800 Subject: [PATCH 2/8] implement decaying of usage counter --- program-runtime/src/loaded_programs.rs | 197 ++++++++++++++++++++++++- runtime/src/bank.rs | 1 + 2 files changed, 195 insertions(+), 3 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 385d68ffbdc7a8..13c60323f9a52e 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -130,6 +130,8 @@ pub struct LoadedProgram { pub tx_usage_counter: AtomicU64, /// How often this entry was used by an instruction pub ix_usage_counter: AtomicU64, + /// Latest slot in which the entry was used + pub latest_access_slot: RwLock, } #[derive(Debug, Default)] @@ -349,6 +351,7 @@ impl LoadedProgram { tx_usage_counter: AtomicU64::new(0), program, ix_usage_counter: AtomicU64::new(0), + latest_access_slot: RwLock::new(0), }) } @@ -361,6 +364,7 @@ impl LoadedProgram { 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: RwLock::new(*self.latest_access_slot.read().unwrap()), }) } @@ -382,6 +386,7 @@ impl LoadedProgram { tx_usage_counter: AtomicU64::new(0), program: LoadedProgramType::Builtin(BuiltinProgram::new_builtin(function_registry)), ix_usage_counter: AtomicU64::new(0), + latest_access_slot: RwLock::new(0), } } @@ -396,6 +401,7 @@ impl LoadedProgram { maybe_expiration_slot, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), + latest_access_slot: RwLock::new(0), }; debug_assert!(tombstone.is_tombstone()); tombstone @@ -418,8 +424,17 @@ impl LoadedProgram { && slot < self.effective_slot } - fn decayed_usage_counter(&self, _now: Slot) -> u64 { - self.tx_usage_counter.load(Ordering::Relaxed) + pub fn update_access_slot(&self, slot: Slot) { + let mut current_slot = self.latest_access_slot.write().unwrap(); + if slot > *current_slot { + *current_slot = slot; + } + } + + fn decayed_usage_counter(&self, now: Slot) -> u64 { + let last_access = self.latest_access_slot.read().unwrap(); + let decaying_for = now.saturating_sub(*last_access); + self.tx_usage_counter.load(Ordering::Relaxed) >> decaying_for } } @@ -867,7 +882,7 @@ impl LoadedPrograms { if let LoadedProgramType::Unloaded(_environment) = &entry.program { break; } - + entry.update_access_slot(loaded_programs_for_tx_batch.slot); entry.clone() } else if entry.is_implicit_delay_visibility_tombstone( loaded_programs_for_tx_batch.slot, @@ -1187,6 +1202,7 @@ mod tests { maybe_expiration_slot: expiry, tx_usage_counter: usage_counter, ix_usage_counter: AtomicU64::default(), + latest_access_slot: RwLock::new(0), }) } @@ -1199,6 +1215,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), + latest_access_slot: RwLock::new(0), }) } @@ -1227,6 +1244,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), + latest_access_slot: RwLock::new(0), } .to_unloaded() .expect("Failed to unload the program"), @@ -1252,6 +1270,176 @@ mod tests { .sum() } + #[test] + fn test_usage_counter_decay() { + let _cache = new_mock_cache::(); + let program = new_test_loaded_program_with_usage(10, 11, AtomicU64::new(32)); + program.update_access_slot(15); + assert_eq!(program.decayed_usage_counter(15), 32); + assert_eq!(program.decayed_usage_counter(16), 16); + assert_eq!(program.decayed_usage_counter(17), 8); + assert_eq!(program.decayed_usage_counter(18), 4); + assert_eq!(program.decayed_usage_counter(19), 2); + assert_eq!(program.decayed_usage_counter(20), 1); + assert_eq!(program.decayed_usage_counter(21), 0); + assert_eq!(program.decayed_usage_counter(15), 32); + assert_eq!(program.decayed_usage_counter(14), 32); + + program.update_access_slot(18); + assert_eq!(program.decayed_usage_counter(15), 32); + assert_eq!(program.decayed_usage_counter(16), 32); + assert_eq!(program.decayed_usage_counter(17), 32); + assert_eq!(program.decayed_usage_counter(18), 32); + assert_eq!(program.decayed_usage_counter(19), 16); + assert_eq!(program.decayed_usage_counter(20), 8); + assert_eq!(program.decayed_usage_counter(21), 4); + } + + #[test] + fn test_random_eviction() { + let mut programs = vec![]; + + let mut cache = new_mock_cache::(); + + let program1 = Pubkey::new_unique(); + let program1_deployment_slots = [0, 10, 20]; + let program1_usage_counters = [4, 5, 25]; + program1_deployment_slots + .iter() + .enumerate() + .for_each(|(i, deployment_slot)| { + let usage_counter = *program1_usage_counters.get(i).unwrap_or(&0); + cache.replenish( + program1, + new_test_loaded_program_with_usage( + *deployment_slot, + (*deployment_slot) + 2, + AtomicU64::new(usage_counter), + ), + ); + programs.push((program1, *deployment_slot, usage_counter)); + }); + + let env = Arc::new(BuiltinProgram::new_mock()); + for slot in 21..31 { + set_tombstone( + &mut cache, + program1, + slot, + LoadedProgramType::FailedVerification(env.clone()), + ); + } + + for slot in 31..41 { + insert_unloaded_program(&mut cache, program1, slot); + } + + let program2 = Pubkey::new_unique(); + let program2_deployment_slots = [5, 11]; + let program2_usage_counters = [0, 2]; + program2_deployment_slots + .iter() + .enumerate() + .for_each(|(i, deployment_slot)| { + let usage_counter = *program2_usage_counters.get(i).unwrap_or(&0); + cache.replenish( + program2, + new_test_loaded_program_with_usage( + *deployment_slot, + (*deployment_slot) + 2, + AtomicU64::new(usage_counter), + ), + ); + programs.push((program2, *deployment_slot, usage_counter)); + }); + + for slot in 21..31 { + set_tombstone( + &mut cache, + program2, + slot, + LoadedProgramType::DelayVisibility, + ); + } + + for slot in 31..41 { + insert_unloaded_program(&mut cache, program2, slot); + } + + let program3 = Pubkey::new_unique(); + let program3_deployment_slots = [0, 5, 15]; + let program3_usage_counters = [100, 3, 20]; + program3_deployment_slots + .iter() + .enumerate() + .for_each(|(i, deployment_slot)| { + let usage_counter = *program3_usage_counters.get(i).unwrap_or(&0); + cache.replenish( + program3, + new_test_loaded_program_with_usage( + *deployment_slot, + (*deployment_slot) + 2, + AtomicU64::new(usage_counter), + ), + ); + programs.push((program3, *deployment_slot, usage_counter)); + }); + + for slot in 21..31 { + set_tombstone(&mut cache, program3, slot, LoadedProgramType::Closed); + } + + for slot in 31..41 { + insert_unloaded_program(&mut cache, program3, slot); + } + + programs.sort_by_key(|(_id, _slot, usage_count)| *usage_count); + + let num_loaded = num_matching_entries(&cache, |program_type| { + matches!(program_type, LoadedProgramType::TestLoaded(_)) + }); + let num_unloaded = num_matching_entries(&cache, |program_type| { + matches!(program_type, LoadedProgramType::Unloaded(_)) + }); + let num_tombstones = num_matching_entries(&cache, |program_type| { + matches!( + program_type, + LoadedProgramType::DelayVisibility + | LoadedProgramType::FailedVerification(_) + | LoadedProgramType::Closed + ) + }); + + assert_eq!(num_loaded, 8); + assert_eq!(num_unloaded, 30); + assert_eq!(num_tombstones, 30); + + // Evicting to 2% should update cache with + // * 5 active entries + // * 33 unloaded entries (3 active programs will get unloaded) + // * 30 tombstones (tombstones are not evicted) + cache.evict_using_2s_random_selection(Percentage::from(2), 50); + + let num_loaded = num_matching_entries(&cache, |program_type| { + matches!(program_type, LoadedProgramType::TestLoaded(_)) + }); + let num_unloaded = num_matching_entries(&cache, |program_type| { + matches!(program_type, LoadedProgramType::Unloaded(_)) + }); + let num_tombstones = num_matching_entries(&cache, |program_type| { + matches!( + program_type, + LoadedProgramType::DelayVisibility + | LoadedProgramType::FailedVerification(_) + | LoadedProgramType::Closed + ) + }); + + assert_eq!(num_loaded, 5); + assert_eq!(num_unloaded, 33); + assert_eq!(num_tombstones, 30); + } + #[test] fn test_eviction() { let mut programs = vec![]; @@ -1644,6 +1832,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), + latest_access_slot: RwLock::new(0), }); let (existing, program) = cache.replenish(program1, updated_program.clone()); assert!(!existing); @@ -1936,6 +2125,7 @@ mod tests { maybe_expiration_slot: Some(21), tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), + latest_access_slot: RwLock::new(0), }); assert!(!cache.replenish(program4, test_program).0); @@ -2279,6 +2469,7 @@ mod tests { maybe_expiration_slot: Some(15), tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), + latest_access_slot: RwLock::new(0), }); assert!(!cache.replenish(program1, test_program).0); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index f8c87e11312ea2..c56d5798c9ac8a 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4799,6 +4799,7 @@ impl Bank { loaded_program.ix_usage_counter = AtomicU64::new(recompile.ix_usage_counter.load(Ordering::Relaxed)); } + loaded_program.update_access_slot(self.slot()); Arc::new(loaded_program) } From a865b4268f4ed5a72de8ed7d53663357ab53a4d1 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Sun, 10 Dec 2023 08:47:07 -0800 Subject: [PATCH 3/8] fix test compilation --- programs/bpf_loader/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 25b2df318f7f19..2ae60909dd2631 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -1696,7 +1696,12 @@ mod tests { rent::Rent, system_program, sysvar, }, - std::{fs::File, io::Read, ops::Range, sync::atomic::AtomicU64}, + std::{ + fs::File, + io::Read, + ops::Range, + sync::{atomic::AtomicU64, RwLock}, + }, }; struct TestContextObject { @@ -4021,6 +4026,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::new(100), ix_usage_counter: AtomicU64::new(100), + latest_access_slot: RwLock::new(0), }; invoke_context .programs_modified_by_tx @@ -4061,6 +4067,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::new(100), ix_usage_counter: AtomicU64::new(100), + latest_access_slot: RwLock::new(0), }; invoke_context .programs_modified_by_tx From 09802dfb5ca350e356e3e61ad6a648d281b1c823 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 11 Dec 2023 10:28:33 -0800 Subject: [PATCH 4/8] replace RwLock with AtomicU64 --- program-runtime/src/loaded_programs.rs | 60 +++++++++++++++++++------- programs/bpf_loader/src/lib.rs | 11 ++--- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 13c60323f9a52e..7b9014be30075d 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -131,7 +131,7 @@ pub struct LoadedProgram { /// How often this entry was used by an instruction pub ix_usage_counter: AtomicU64, /// Latest slot in which the entry was used - pub latest_access_slot: RwLock, + pub latest_access_slot: AtomicU64, } #[derive(Debug, Default)] @@ -351,7 +351,7 @@ impl LoadedProgram { tx_usage_counter: AtomicU64::new(0), program, ix_usage_counter: AtomicU64::new(0), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::new(0), }) } @@ -364,7 +364,7 @@ impl LoadedProgram { 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: RwLock::new(*self.latest_access_slot.read().unwrap()), + latest_access_slot: AtomicU64::new(self.latest_access_slot.load(Ordering::Relaxed)), }) } @@ -386,7 +386,7 @@ impl LoadedProgram { tx_usage_counter: AtomicU64::new(0), program: LoadedProgramType::Builtin(BuiltinProgram::new_builtin(function_registry)), ix_usage_counter: AtomicU64::new(0), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::new(0), } } @@ -401,7 +401,7 @@ impl LoadedProgram { maybe_expiration_slot, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::new(0), }; debug_assert!(tombstone.is_tombstone()); tombstone @@ -425,15 +425,24 @@ impl LoadedProgram { } pub fn update_access_slot(&self, slot: Slot) { - let mut current_slot = self.latest_access_slot.write().unwrap(); - if slot > *current_slot { - *current_slot = slot; + let mut last_access = self.latest_access_slot.load(Ordering::Relaxed); + while last_access < slot { + if let Err(updated_last_access) = self.latest_access_slot.compare_exchange( + last_access, + slot, + Ordering::Relaxed, + Ordering::Relaxed, + ) { + last_access = updated_last_access; + } else { + last_access = slot; + } } } fn decayed_usage_counter(&self, now: Slot) -> u64 { - let last_access = self.latest_access_slot.read().unwrap(); - let decaying_for = now.saturating_sub(*last_access); + let last_access = self.latest_access_slot.load(Ordering::Relaxed); + let decaying_for = now.saturating_sub(last_access); self.tx_usage_counter.load(Ordering::Relaxed) >> decaying_for } } @@ -1066,8 +1075,15 @@ impl LoadedPrograms { .entry(*id) .and_modify(|c| saturating_add_assign!(*c, 1)) .or_insert(1); + } else { + error!( + "Failed to create an unloaded cache entry for a program type {:?}", + entry.program + ); } } + } else { + error!("Failed to find a cached entry for program {}", id); } } @@ -1093,8 +1109,20 @@ impl LoadedPrograms { .and_modify(|c| saturating_add_assign!(*c, 1)) .or_insert(1); *candidate = Arc::new(unloaded); + } else { + error!( + "Failed to create an unloaded cache entry for a program type {:?}", + candidate.program + ); } + } else { + error!( + "Failed to find the cache entry for program {} that was to be unloaded", + program + ); } + } else { + error!("Failed to find a cached entry for program {}", program); } } @@ -1202,7 +1230,7 @@ mod tests { maybe_expiration_slot: expiry, tx_usage_counter: usage_counter, ix_usage_counter: AtomicU64::default(), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::default(), }) } @@ -1215,7 +1243,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::default(), }) } @@ -1244,7 +1272,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::default(), } .to_unloaded() .expect("Failed to unload the program"), @@ -1832,7 +1860,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::default(), }); let (existing, program) = cache.replenish(program1, updated_program.clone()); assert!(!existing); @@ -2125,7 +2153,7 @@ mod tests { maybe_expiration_slot: Some(21), tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::default(), }); assert!(!cache.replenish(program4, test_program).0); @@ -2469,7 +2497,7 @@ mod tests { maybe_expiration_slot: Some(15), tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::default(), }); assert!(!cache.replenish(program1, test_program).0); diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 2ae60909dd2631..281d314cb4b5b4 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -1696,12 +1696,7 @@ mod tests { rent::Rent, system_program, sysvar, }, - std::{ - fs::File, - io::Read, - ops::Range, - sync::{atomic::AtomicU64, RwLock}, - }, + std::{fs::File, io::Read, ops::Range, sync::atomic::AtomicU64}, }; struct TestContextObject { @@ -4026,7 +4021,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::new(100), ix_usage_counter: AtomicU64::new(100), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::new(0), }; invoke_context .programs_modified_by_tx @@ -4067,7 +4062,7 @@ mod tests { maybe_expiration_slot: None, tx_usage_counter: AtomicU64::new(100), ix_usage_counter: AtomicU64::new(100), - latest_access_slot: RwLock::new(0), + latest_access_slot: AtomicU64::new(0), }; invoke_context .programs_modified_by_tx From 0678afc229f460be5ef3d6dde24945f0a2700056 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 11 Dec 2023 21:07:50 -0800 Subject: [PATCH 5/8] address review comments --- program-runtime/src/loaded_programs.rs | 96 ++++++++++++-------------- 1 file changed, 46 insertions(+), 50 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 7b9014be30075d..20ad045320c104 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -1032,29 +1032,30 @@ impl LoadedPrograms { let num_to_unload = candidates .len() .saturating_sub(shrink_to.apply_to(MAX_LOADED_ENTRY_COUNT)); - for _ in 0..num_to_unload { + fn random_index_and_usage_counter( + candidates: &[(Pubkey, Arc)], + now: Slot, + ) -> (usize, u64) { let mut rng = thread_rng(); - let index1 = rng.gen_range(0..candidates.len()); - let index2 = rng.gen_range(0..candidates.len()); - if let Some((usage_counter1, usage_counter2)) = - candidates.get(index1).and_then(|(_id, entry1)| { - candidates.get(index2).map(|(_id, entry2)| { - ( - entry1.decayed_usage_counter(now), - entry2.decayed_usage_counter(now), - ) - }) - }) - { - let (program, entry) = if usage_counter1 < usage_counter2 { - candidates.remove(index1) - } else { - candidates.remove(index2) - }; - self.unload_program_entry(&program, &entry); + let index = rng.gen_range(0..candidates.len()); + let usage_counter = candidates + .get(index) + .expect("Failed to get cached entry") + .1 + .decayed_usage_counter(now); + (index, usage_counter) + } + + for _ in 0..num_to_unload { + let (index1, usage_counter1) = random_index_and_usage_counter(&candidates, now); + let (index2, usage_counter2) = random_index_and_usage_counter(&candidates, now); + + let (program, entry) = if usage_counter1 < usage_counter2 { + candidates.remove(index1) } else { - error!("Failed in evicting an entry from the program cache"); - } + candidates.remove(index2) + }; + self.unload_program_entry(&program, &entry); } } @@ -1093,36 +1094,26 @@ impl LoadedPrograms { } fn unload_program_entry(&mut self, program: &Pubkey, remove_entry: &Arc) { - if let Some(second_level) = self.entries.get_mut(program) { - if let Some(candidate) = second_level - .slot_versions - .iter_mut() - .find(|entry| entry == &remove_entry) - { - 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); - } - self.stats - .evictions - .entry(*program) - .and_modify(|c| saturating_add_assign!(*c, 1)) - .or_insert(1); - *candidate = Arc::new(unloaded); - } else { - error!( - "Failed to create an unloaded cache entry for a program type {:?}", - candidate.program - ); - } - } else { - error!( - "Failed to find the cache entry for program {} that was to be unloaded", - program - ); + let second_level = self.entries.get_mut(program).expect("Cache lookup failed"); + let candidate = second_level + .slot_versions + .iter_mut() + .find(|entry| entry == &remove_entry) + .expect("Program entry not found"); + + // Certain entry types cannot be unloaded, such as tombstones, or already unloaded entries. + // For such entries, `to_unloaded()` will return None. + // These entry types do not occupy much memory. + 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); } - } else { - error!("Failed to find a cached entry for program {}", program); + self.stats + .evictions + .entry(*program) + .and_modify(|c| saturating_add_assign!(*c, 1)) + .or_insert(1); + *candidate = Arc::new(unloaded); } } @@ -1329,6 +1320,9 @@ mod tests { let mut cache = new_mock_cache::(); + // This test adds different kind of entries to the cache. + // Tombstones and unloaded entries are expected to not be evicted. + // It also adds multiple entries for three programs as it tries to create a typical cache instance. let program1 = Pubkey::new_unique(); let program1_deployment_slots = [0, 10, 20]; let program1_usage_counters = [4, 5, 25]; @@ -1438,6 +1432,7 @@ mod tests { ) }); + // Test that the cache is constructed with the expected number of entries. assert_eq!(num_loaded, 8); assert_eq!(num_unloaded, 30); assert_eq!(num_tombstones, 30); @@ -1463,6 +1458,7 @@ mod tests { ) }); + // Test that expected number of loaded entries get evicted/unloaded. assert_eq!(num_loaded, 5); assert_eq!(num_unloaded, 33); assert_eq!(num_tombstones, 30); From 9961580e98545f650454b761020a661298e9eb52 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 12 Dec 2023 06:21:20 -0800 Subject: [PATCH 6/8] address more review comments --- program-runtime/src/loaded_programs.rs | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 20ad045320c104..a16d8ec49154c6 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -425,19 +425,11 @@ impl LoadedProgram { } pub fn update_access_slot(&self, slot: Slot) { - let mut last_access = self.latest_access_slot.load(Ordering::Relaxed); - while last_access < slot { - if let Err(updated_last_access) = self.latest_access_slot.compare_exchange( - last_access, - slot, - Ordering::Relaxed, - Ordering::Relaxed, - ) { - last_access = updated_last_access; - } else { - last_access = slot; - } - } + let _ = self.latest_access_slot.fetch_update( + Ordering::Relaxed, + Ordering::Relaxed, + |last_access| (last_access < slot).then_some(slot), + ); } fn decayed_usage_counter(&self, now: Slot) -> u64 { @@ -1083,8 +1075,6 @@ impl LoadedPrograms { ); } } - } else { - error!("Failed to find a cached entry for program {}", id); } } @@ -1093,6 +1083,8 @@ impl LoadedPrograms { keys.iter().for_each(|key| self.unload_program(key)); } + /// This function removes the given entry for the given program from the cache. + /// The function expects that the program and entry exists in the cache. Otherwise it'll panic. fn unload_program_entry(&mut self, program: &Pubkey, remove_entry: &Arc) { let second_level = self.entries.get_mut(program).expect("Cache lookup failed"); let candidate = second_level @@ -1221,7 +1213,7 @@ mod tests { maybe_expiration_slot: expiry, tx_usage_counter: usage_counter, ix_usage_counter: AtomicU64::default(), - latest_access_slot: AtomicU64::default(), + latest_access_slot: AtomicU64::new(deployment_slot), }) } @@ -1441,7 +1433,7 @@ mod tests { // * 5 active entries // * 33 unloaded entries (3 active programs will get unloaded) // * 30 tombstones (tombstones are not evicted) - cache.evict_using_2s_random_selection(Percentage::from(2), 50); + cache.evict_using_2s_random_selection(Percentage::from(2), 21); let num_loaded = num_matching_entries(&cache, |program_type| { matches!(program_type, LoadedProgramType::TestLoaded(_)) From b790e74dfff6d75741e55c780c82a5b3a680b398 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 13 Dec 2023 03:32:42 -0800 Subject: [PATCH 7/8] remove -> swap_remove --- program-runtime/src/loaded_programs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index a16d8ec49154c6..37044da622d80b 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -1043,9 +1043,9 @@ impl LoadedPrograms { let (index2, usage_counter2) = random_index_and_usage_counter(&candidates, now); let (program, entry) = if usage_counter1 < usage_counter2 { - candidates.remove(index1) + candidates.swap_remove(index1) } else { - candidates.remove(index2) + candidates.swap_remove(index2) }; self.unload_program_entry(&program, &entry); } From 1d30f3b6dd6c96ea8ac80655fb42abd04fa67cde Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 18 Dec 2023 11:42:17 -0800 Subject: [PATCH 8/8] more review comments --- program-runtime/src/loaded_programs.rs | 42 +++++--------------------- runtime/src/bank.rs | 8 ++--- 2 files changed, 12 insertions(+), 38 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 37044da622d80b..b7b92a0409c800 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -3,7 +3,6 @@ use { invoke_context::{BuiltinFunctionWithContext, InvokeContext}, timings::ExecuteDetailsTimings, }, - itertools::Itertools, log::{debug, error, log_enabled, trace}, percentage::PercentageInteger, rand::{thread_rng, Rng}, @@ -425,14 +424,10 @@ impl LoadedProgram { } pub fn update_access_slot(&self, slot: Slot) { - let _ = self.latest_access_slot.fetch_update( - Ordering::Relaxed, - Ordering::Relaxed, - |last_access| (last_access < slot).then_some(slot), - ); + let _ = self.latest_access_slot.fetch_max(slot, Ordering::Relaxed); } - fn decayed_usage_counter(&self, now: Slot) -> u64 { + pub fn decayed_usage_counter(&self, now: Slot) -> u64 { let last_access = self.latest_access_slot.load(Ordering::Relaxed); let decaying_for = now.saturating_sub(last_access); self.tx_usage_counter.load(Ordering::Relaxed) >> decaying_for @@ -883,7 +878,6 @@ impl LoadedPrograms { if let LoadedProgramType::Unloaded(_environment) = &entry.program { break; } - entry.update_access_slot(loaded_programs_for_tx_batch.slot); entry.clone() } else if entry.is_implicit_delay_visibility_tombstone( loaded_programs_for_tx_batch.slot, @@ -898,6 +892,7 @@ impl LoadedPrograms { } else { continue; }; + entry_to_return.update_access_slot(loaded_programs_for_tx_batch.slot); entry_to_return .tx_usage_counter .fetch_add(*usage_count, Ordering::Relaxed); @@ -957,29 +952,7 @@ impl LoadedPrograms { } /// Returns the list of loaded programs which are verified and compiled. - fn get_flattened_entries(&self) -> Vec<(Pubkey, Arc)> { - self.entries - .iter() - .flat_map(|(id, second_level)| { - second_level - .slot_versions - .iter() - .filter_map(move |program| match program.program { - LoadedProgramType::LegacyV0(_) - | LoadedProgramType::LegacyV1(_) - | LoadedProgramType::Typed(_) => Some((*id, program.clone())), - #[cfg(test)] - LoadedProgramType::TestLoaded(_) => Some((*id, program.clone())), - _ => None, - }) - }) - .collect() - } - - /// Returns the list of loaded programs which are verified and compiled sorted by `tx_usage_counter`. - /// - /// Entries from program runtime v1 and v2 can be individually filtered. - pub fn get_entries_sorted_by_tx_usage( + pub fn get_flattened_entries( &self, include_program_runtime_v1: bool, include_program_runtime_v2: bool, @@ -1004,13 +977,14 @@ impl LoadedPrograms { _ => None, }) }) - .sorted_by_cached_key(|(_id, program)| program.tx_usage_counter.load(Ordering::Relaxed)) .collect() } /// Unloads programs which were used infrequently pub fn sort_and_unload(&mut self, shrink_to: PercentageInteger) { - let sorted_candidates = self.get_entries_sorted_by_tx_usage(true, true); + let mut sorted_candidates = self.get_flattened_entries(true, true); + sorted_candidates + .sort_by_cached_key(|(_id, program)| program.tx_usage_counter.load(Ordering::Relaxed)); let num_to_unload = sorted_candidates .len() .saturating_sub(shrink_to.apply_to(MAX_LOADED_ENTRY_COUNT)); @@ -1020,7 +994,7 @@ impl LoadedPrograms { /// Evicts programs using 2's random selection, choosing the least used program out of the two entries. /// The eviction is performed enough number of times to reduce the cache usage to the given percentage. pub fn evict_using_2s_random_selection(&mut self, shrink_to: PercentageInteger, now: Slot) { - let mut candidates = self.get_flattened_entries(); + let mut candidates = self.get_flattened_entries(true, true); let num_to_unload = candidates .len() .saturating_sub(shrink_to.apply_to(MAX_LOADED_ENTRY_COUNT)); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c56d5798c9ac8a..b865c86d2b35d3 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1476,10 +1476,10 @@ impl Bank { } loaded_programs_cache.upcoming_environments = Some(upcoming_environments); loaded_programs_cache.programs_to_recompile = loaded_programs_cache - .get_entries_sorted_by_tx_usage( - changed_program_runtime_v1, - changed_program_runtime_v2, - ); + .get_flattened_entries(changed_program_runtime_v1, changed_program_runtime_v2); + loaded_programs_cache + .programs_to_recompile + .sort_by_cached_key(|(_id, program)| program.decayed_usage_counter(slot)); } });