From dde06bd7fefe95b85f6000c466fc6a65cb90e8f8 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 3 Apr 2023 19:17:48 -0700 Subject: [PATCH 1/6] Add provisions to unload a program from the cache --- program-runtime/src/loaded_programs.rs | 257 ++++++++----------------- runtime/src/bank.rs | 2 +- 2 files changed, 76 insertions(+), 183 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index c6b20d64f9dc87..27c416d2d6c641 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -61,6 +61,7 @@ pub enum LoadedProgramType { FailedVerification, Closed, DelayVisibility, + Unloaded, LegacyV0(VerifiedExecutable>), LegacyV1(VerifiedExecutable>), Typed(VerifiedExecutable>), @@ -75,6 +76,7 @@ impl Debug for LoadedProgramType { } LoadedProgramType::Closed => write!(f, "LoadedProgramType::Closed"), LoadedProgramType::DelayVisibility => write!(f, "LoadedProgramType::DelayVisibility"), + LoadedProgramType::Unloaded => write!(f, "LoadedProgramType::Unloaded"), LoadedProgramType::LegacyV0(_) => write!(f, "LoadedProgramType::LegacyV0"), LoadedProgramType::LegacyV1(_) => write!(f, "LoadedProgramType::LegacyV1"), LoadedProgramType::Typed(_) => write!(f, "LoadedProgramType::Typed"), @@ -128,6 +130,14 @@ impl LoadProgramMetrics { } } +impl PartialEq for LoadedProgram { + fn eq(&self, other: &Self) -> bool { + self.effective_slot == other.effective_slot + && self.deployment_slot == other.deployment_slot + && self.is_tombstone() == other.is_tombstone() + } +} + impl LoadedProgram { /// Creates a new user program pub fn new( @@ -187,6 +197,16 @@ impl LoadedProgram { }) } + pub fn to_unloaded(&self) -> Self { + Self { + program: LoadedProgramType::Unloaded, + account_size: 0, + deployment_slot: self.deployment_slot, + effective_slot: self.effective_slot, + usage_counter: AtomicU64::new(self.usage_counter.load(Ordering::Relaxed)), + } + } + /// Creates a new built-in program pub fn new_built_in( deployment_slot: Slot, @@ -345,35 +365,39 @@ impl LoadedPrograms { (found, missing) } - /// Evicts programs which were used infrequently - pub fn sort_and_evict(&mut self, max_cache_entries: Option) { + /// Unloads programs which were used infrequently + pub fn sort_and_unload(&mut self, max_cache_entries: Option) { // Find eviction candidates and sort by their usage counters - let mut num_cache_entries: usize = 0; let sorted_candidates = self .entries .iter() - .filter(|(_key, programs)| { - num_cache_entries = num_cache_entries.saturating_add(programs.len()); - programs.len() == 1 - }) - .sorted_by_cached_key(|(_key, programs)| { - programs - .get(0) - .unwrap() - .usage_counter - .load(Ordering::Relaxed) + .flat_map(|(id, list)| { + list.iter().filter_map(|program| match program.program { + LoadedProgramType::LegacyV0(_) + | LoadedProgramType::LegacyV1(_) + | LoadedProgramType::Typed(_) + | LoadedProgramType::BuiltIn(_) => Some((*id, program.clone())), + _ => None, + }) }) - .map(|(key, _programs)| *key) - .collect::>(); - // Calculate how many to remove - let num_to_remove = std::cmp::min( - num_cache_entries.saturating_sub(max_cache_entries.unwrap_or(MAX_CACHE_ENTRIES)), - sorted_candidates.len(), - ); - // Remove selected entries - if num_to_remove != 0 { - self.remove_entries(sorted_candidates.into_iter().take(num_to_remove)) - } + .sorted_by_cached_key(|(_, program)| program.usage_counter.load(Ordering::Relaxed)) + .collect::)>>(); + + // Calculate how many to unload + let num_to_unload = sorted_candidates + .len() + .saturating_sub(max_cache_entries.unwrap_or(MAX_CACHE_ENTRIES)); + sorted_candidates + .iter() + .take(num_to_unload) + .for_each(|(id, program)| { + self.entries.get_mut(id).map(|entries| { + entries + .iter_mut() + .find(|entry| entry == &program) + .map(|candidate| *candidate = Arc::new(candidate.to_unloaded())); + }); + }); } /// Removes the entries at the given keys, if they exist @@ -439,9 +463,7 @@ mod tests { fork_graph.insert_fork(&[0, 10, 20, 22]); fork_graph.insert_fork(&[0, 5, 11, 15, 16]); fork_graph.insert_fork(&[0, 5, 11, 25, 27]); - let possible_slots: Vec = vec![0, 5, 10, 11, 15, 16, 20, 22, 25, 27]; - let usage_counters: Vec = vec![43, 10, 1128, 1, 0, 67, 212, 322, 29, 21]; - let mut programs = HashMap::>::new(); + let mut programs = vec![]; let mut num_total_programs: usize = 0; let mut cache = LoadedPrograms::default(); @@ -453,29 +475,17 @@ mod tests { .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(*program1_usage_counters.get(i).unwrap_or(&0)), + AtomicU64::new(usage_counter), ), ); num_total_programs += 1; - programs - .entry(program1) - .and_modify(|entries| { - entries.push(( - *deployment_slot, - *program1_usage_counters.get(i).unwrap_or(&0), - )) - }) - .or_insert_with(|| { - Vec::<(u64, u64)>::from([( - *deployment_slot, - *program1_usage_counters.get(i).unwrap_or(&0), - )]) - }); + programs.push((program1, *deployment_slot, usage_counter)); }); let program2 = Pubkey::new_unique(); @@ -485,29 +495,17 @@ mod tests { .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(*program2_usage_counters.get(i).unwrap_or(&0)), + AtomicU64::new(usage_counter), ), ); num_total_programs += 1; - programs - .entry(program2) - .and_modify(|entries| { - entries.push(( - *deployment_slot, - *program2_usage_counters.get(i).unwrap_or(&0), - )) - }) - .or_insert_with(|| { - Vec::<(u64, u64)>::from([( - *deployment_slot, - *program2_usage_counters.get(i).unwrap_or(&0), - )]) - }); + programs.push((program2, *deployment_slot, usage_counter)); }); let program3 = Pubkey::new_unique(); @@ -517,147 +515,42 @@ mod tests { .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(*program3_usage_counters.get(i).unwrap_or(&0)), + AtomicU64::new(usage_counter), ), ); num_total_programs += 1; - programs - .entry(program3) - .and_modify(|entries| { - entries.push(( - *deployment_slot, - *program3_usage_counters.get(i).unwrap_or(&0), - )) - }) - .or_insert_with(|| { - Vec::<(u64, u64)>::from([( - *deployment_slot, - *program3_usage_counters.get(i).unwrap_or(&0), - )]) - }); + programs.push((program3, *deployment_slot, usage_counter)); }); - // Add random set of used programs (with no redeploys) on each possible slot - // in the fork graph - let mut eviction_candidates = possible_slots - .into_iter() - .enumerate() - .map(|(i, slot)| { - ( - Pubkey::new_unique(), - slot, - *usage_counters.get(i).unwrap_or(&0), - ) - }) - .collect::>(); - eviction_candidates - .iter() - .for_each(|(key, deployment_slot, usage_counter)| { - cache.replenish( - *key, - new_test_loaded_program_with_usage( - *deployment_slot, - (*deployment_slot) + 2, - AtomicU64::new(*usage_counter), - ), - ); - num_total_programs += 1; - programs - .entry(*key) - .and_modify(|entries| entries.push((*deployment_slot, *usage_counter))) - .or_insert_with(|| { - Vec::<(u64, u64)>::from([(*deployment_slot, *usage_counter)]) - }); - }); - eviction_candidates.sort_by_key(|(_key, _deplyment_slot, usage_counter)| *usage_counter); + programs.sort_by_key(|(_id, _slot, usage_count)| *usage_count); - // Try to remove no programs. - cache.sort_and_evict(Some(num_total_programs)); + cache.sort_and_unload(Some(num_total_programs.saturating_sub(3))); // Check that every program is still in the cache. programs.iter().for_each(|entry| { - assert!(cache.entries.get(entry.0).is_some()); + assert!(cache.entries.get(&entry.0).is_some()); }); - // Try to remove less than max programs. - let max_cache_entries = 12_usize; - // Guarantee you won't evict all eviction candidates - let num_to_remove = num_total_programs - max_cache_entries; - assert!(eviction_candidates.len() > num_to_remove); - let removals = eviction_candidates - .drain(0..num_to_remove) - .map(|(key, _, _)| key) - .collect::>(); - cache.sort_and_evict(Some(max_cache_entries)); - // Make sure removed entries are gone - removals.iter().for_each(|key| { - assert!(cache.entries.get(key).is_none()); - }); - // Make sure the other entries are still present in the cache - programs + let unloaded = cache + .entries .iter() - .filter(|(key, _)| !removals.contains(key)) - .for_each( - // For every entry not removed - |(key, val)| { - let program_in_cache = cache.entries.get(key); - assert!(program_in_cache.is_some()); // Make sure it's entry exists - let values_in_cache = program_in_cache - .unwrap() - .iter() - .map(|x| (x.deployment_slot, x.usage_counter.load(Ordering::Relaxed))) - .collect::>(); - val.iter().for_each(|entry| { - // make sure the exact slot and usage counter remain - // for the entry - assert!(values_in_cache.contains(entry)); - }); - }, - ); - // Remove entries from you local cache tracker - removals.iter().for_each(|key| { - programs.remove(key); - num_total_programs -= 1; - }); + .flat_map(|(id, cached_programs)| { + cached_programs.iter().filter_map(|program| { + matches!(program.program, LoadedProgramType::Unloaded) + .then_some((*id, program.usage_counter.load(Ordering::Relaxed))) + }) + }) + .collect::>(); - // Try to remove all programs. - let max_num_removals = eviction_candidates.len(); - // Make sure total programs is greater than number of eviction candidates - assert!(num_total_programs > max_num_removals); - cache.sort_and_evict(Some(0)); - // Make sure all candidate removals were removed - let removals = eviction_candidates - .iter() - .map(|(key, _, _)| key) - .collect::>(); - removals.iter().for_each(|key| { - assert!(cache.entries.get(*key).is_none()); - }); - // Make sure all non-candidate removals remain - programs - .iter() - .filter(|(key, _)| !removals.contains(key)) - .for_each( - // For every entry not removed - |(key, val)| { - let program_in_cache = cache.entries.get(key); - assert!(program_in_cache.is_some()); // Make sure it's entry exists - let values_in_cache = program_in_cache - .unwrap() - .iter() - .map(|x| (x.deployment_slot, x.usage_counter.load(Ordering::Relaxed))) - .collect::>(); - val.iter().for_each(|entry| { - // make sure the exact slot and usage counter remain - // for the entry - assert!(values_in_cache.contains(entry)); - }); - }, - ); + for index in 0..3 { + let expected = programs.get(index).expect("Missing program"); + assert!(unloaded.contains(&(expected.0, expected.2))); + } } #[test] @@ -864,7 +757,7 @@ mod tests { usage_counter: AtomicU64, ) -> Arc { Arc::new(LoadedProgram { - program: LoadedProgramType::FailedVerification, + program: LoadedProgramType::BuiltIn(BuiltInProgram::default()), account_size: 0, deployment_slot, effective_slot, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c075507cb9b818..c0e59ff9815e23 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4644,7 +4644,7 @@ impl Bank { self.loaded_programs_cache .write() .unwrap() - .sort_and_evict(None); + .sort_and_unload(None); debug!( "check: {}us load: {}us execute: {}us txs_len={}", From 8d6fa46903269f53a5ac9d137b8be75dd1c1b84e Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Fri, 7 Apr 2023 04:15:44 +0530 Subject: [PATCH 2/6] evict unloaded, tombstones and expired programs --- Cargo.lock | 2 + program-runtime/Cargo.toml | 1 + program-runtime/src/loaded_programs.rs | 160 +++++++++++++++++++------ runtime/Cargo.toml | 1 + runtime/src/bank.rs | 4 +- 5 files changed, 130 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ead103b6ee806a..d04a0908a55515 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6346,6 +6346,7 @@ dependencies = [ "log", "num-derive", "num-traits", + "percentage", "rand 0.7.3", "rustc_version 0.4.0", "serde", @@ -6647,6 +6648,7 @@ dependencies = [ "num_cpus", "once_cell", "ouroboros", + "percentage", "rand 0.7.3", "rand_chacha 0.2.2", "rayon", diff --git a/program-runtime/Cargo.toml b/program-runtime/Cargo.toml index 2278bdb631eaf4..61e6766cd67eac 100644 --- a/program-runtime/Cargo.toml +++ b/program-runtime/Cargo.toml @@ -19,6 +19,7 @@ libc = { workspace = true } log = { workspace = true } num-derive = { workspace = true } num-traits = { workspace = true } +percentage = { workspace = true } rand = { workspace = true } serde = { version = "1.0.158", features = ["derive", "rc"] } solana-frozen-abi = { workspace = true } diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 27c416d2d6c641..b1f2927d9f1714 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -1,6 +1,7 @@ use { crate::{invoke_context::InvokeContext, timings::ExecuteDetailsTimings}, itertools::Itertools, + percentage::PercentageInteger, solana_measure::measure::Measure, solana_rbpf::{ elf::Executable, @@ -22,7 +23,9 @@ use { }, }; -const MAX_CACHE_ENTRIES: usize = 100; // TODO: Tune to size +const MAX_LOADED_ENTRY_COUNT: usize = 256; +const MAX_UNLOADED_ENTRY_COUNT: usize = 512; +const MAX_TOMBSTONE_COUNT: usize = 512; /// Relationship between two fork IDs #[derive(Copy, Clone, PartialEq)] @@ -65,6 +68,8 @@ pub enum LoadedProgramType { LegacyV0(VerifiedExecutable>), LegacyV1(VerifiedExecutable>), Typed(VerifiedExecutable>), + #[cfg(test)] + TestLoaded, BuiltIn(BuiltInProgram>), } @@ -80,6 +85,8 @@ impl Debug for LoadedProgramType { LoadedProgramType::LegacyV0(_) => write!(f, "LoadedProgramType::LegacyV0"), LoadedProgramType::LegacyV1(_) => write!(f, "LoadedProgramType::LegacyV1"), LoadedProgramType::Typed(_) => write!(f, "LoadedProgramType::Typed"), + #[cfg(test)] + LoadedProgramType::TestLoaded => write!(f, "LoadedProgramType::TestLoaded"), LoadedProgramType::BuiltIn(_) => write!(f, "LoadedProgramType::BuiltIn"), } } @@ -200,9 +207,10 @@ impl LoadedProgram { pub fn to_unloaded(&self) -> Self { Self { program: LoadedProgramType::Unloaded, - account_size: 0, + account_size: self.account_size, deployment_slot: self.deployment_slot, effective_slot: self.effective_slot, + maybe_expiration_slot: self.maybe_expiration_slot, usage_counter: AtomicU64::new(self.usage_counter.load(Ordering::Relaxed)), } } @@ -278,11 +286,18 @@ impl LoadedPrograms { let index = second_level .iter() .position(|at| at.effective_slot >= entry.effective_slot); - if let Some(existing) = index.and_then(|index| second_level.get(index)) { + if let Some((existing, entry_index)) = + index.and_then(|index| second_level.get(index).map(|value| (value, index))) + { if existing.deployment_slot == entry.deployment_slot && existing.effective_slot == entry.effective_slot { - return (true, existing.clone()); + if matches!(existing.program, LoadedProgramType::Unloaded) { + // The unloaded program is getting reloaded + second_level.swap_remove(entry_index); + } else { + return (true, existing.clone()); + } } } second_level.insert(index.unwrap_or(second_level.len()), entry.clone()); @@ -322,6 +337,8 @@ impl LoadedPrograms { second_level.reverse(); !second_level.is_empty() }); + + self.remove_expired_entries(new_root); } /// Extracts a subset of the programs relevant to a transaction batch @@ -365,47 +382,115 @@ impl LoadedPrograms { (found, missing) } - /// Unloads programs which were used infrequently - pub fn sort_and_unload(&mut self, max_cache_entries: Option) { - // Find eviction candidates and sort by their usage counters - let sorted_candidates = self + /// Evicts programs which were used infrequently + pub fn sort_and_evict(&mut self, shrink_to: PercentageInteger) { + let mut num_loaded: usize = 0; + let mut num_unloaded: usize = 0; + let mut num_tombstones: usize = 0; + // Find eviction candidates and sort by their type and usage counters. + // Sorted result will have the following order: + // Loaded entries with ascending order of their usage count + // Unloaded entries with ascending order of their usage count + // Tombstones with ascending order of their usage count + let (ordering, sorted_candidates): (Vec, Vec<(Pubkey, Arc)>) = self .entries .iter() .flat_map(|(id, list)| { - list.iter().filter_map(|program| match program.program { - LoadedProgramType::LegacyV0(_) - | LoadedProgramType::LegacyV1(_) - | LoadedProgramType::Typed(_) - | LoadedProgramType::BuiltIn(_) => Some((*id, program.clone())), - _ => None, - }) + list.iter() + .filter_map(move |program| match program.program { + LoadedProgramType::LegacyV0(_) + | LoadedProgramType::LegacyV1(_) + | LoadedProgramType::Typed(_) => Some((0, (*id, program.clone()))), + #[cfg(test)] + LoadedProgramType::TestLoaded => Some((0, (*id, program.clone()))), + LoadedProgramType::Unloaded => Some((1, (*id, program.clone()))), + LoadedProgramType::FailedVerification + | LoadedProgramType::Closed + | LoadedProgramType::DelayVisibility => Some((2, (*id, program.clone()))), + LoadedProgramType::BuiltIn(_) => None, + }) }) - .sorted_by_cached_key(|(_, program)| program.usage_counter.load(Ordering::Relaxed)) - .collect::)>>(); - - // Calculate how many to unload - let num_to_unload = sorted_candidates - .len() - .saturating_sub(max_cache_entries.unwrap_or(MAX_CACHE_ENTRIES)); - sorted_candidates - .iter() - .take(num_to_unload) - .for_each(|(id, program)| { - self.entries.get_mut(id).map(|entries| { - entries - .iter_mut() - .find(|entry| entry == &program) - .map(|candidate| *candidate = Arc::new(candidate.to_unloaded())); - }); - }); + .sorted_by_cached_key(|(order, (_id, program))| { + (*order, program.usage_counter.load(Ordering::Relaxed)) + }) + .unzip(); + + for order in ordering { + if order == 0 { + num_loaded = num_loaded.saturating_add(1); + } else if order == 1 { + num_unloaded = num_unloaded.saturating_add(1); + } else if order == 2 { + num_tombstones = num_tombstones.saturating_add(1); + } + } + + let num_to_unload = num_loaded.saturating_sub(shrink_to.apply_to(MAX_LOADED_ENTRY_COUNT)); + self.unload_program_entries(sorted_candidates.iter().take(num_to_unload)); + + let num_unloaded_to_evict = + num_unloaded.saturating_sub(shrink_to.apply_to(MAX_UNLOADED_ENTRY_COUNT)); + let (sorted_candidates, unloaded) = sorted_candidates.split_at(num_loaded); + self.remove_program_entries(unloaded.iter().take(num_unloaded_to_evict)); + + let num_tombstones_to_evict = + num_tombstones.saturating_sub(shrink_to.apply_to(MAX_TOMBSTONE_COUNT)); + let (_sorted_candidates, tombstones) = sorted_candidates.split_at(num_unloaded); + self.remove_program_entries(tombstones.iter().take(num_tombstones_to_evict)); + + self.remove_programs_with_no_entries(); } - /// Removes the entries at the given keys, if they exist - pub fn remove_entries(&mut self, keys: impl Iterator) { + /// Removes all the entries at the given keys, if they exist + pub fn remove_programs(&mut self, keys: impl Iterator) { for k in keys { self.entries.remove(&k); } } + + fn remove_program_entries<'a>( + &mut self, + remove: impl Iterator)>, + ) { + for (id, program) in remove { + if let Some(entries) = self.entries.get_mut(id) { + let index = entries.iter().position(|entry| entry == program); + if let Some(index) = index { + entries.swap_remove(index); + } + } + } + } + + fn remove_expired_entries(&mut self, current_slot: Slot) { + for entry in self.entries.values_mut() { + entry.retain(|program| { + program + .maybe_expiration_slot + .map(|expiration| expiration > current_slot) + .unwrap_or(true) + }); + } + self.remove_programs_with_no_entries(); + } + + fn unload_program_entries<'a>( + &mut self, + remove: impl Iterator)>, + ) { + for (id, program) in remove { + if let Some(entries) = self.entries.get_mut(id) { + entries + .iter_mut() + .find(|entry| entry == &program) + .map(|candidate| *candidate = Arc::new(candidate.to_unloaded())); + } + } + } + + fn remove_programs_with_no_entries(&mut self) { + self.entries.retain(|_, programs| !programs.is_empty()) + } } #[cfg(test)] @@ -414,6 +499,7 @@ mod tests { crate::loaded_programs::{ BlockRelation, ForkGraph, LoadedProgram, LoadedProgramType, LoadedPrograms, WorkingSlot, }, + percentage::Percentage, solana_rbpf::vm::BuiltInProgram, solana_sdk::{clock::Slot, pubkey::Pubkey}, std::{ @@ -530,7 +616,7 @@ mod tests { programs.sort_by_key(|(_id, _slot, usage_count)| *usage_count); - cache.sort_and_unload(Some(num_total_programs.saturating_sub(3))); + cache.sort_and_evict(Percentage::from(2)); // Check that every program is still in the cache. programs.iter().for_each(|entry| { assert!(cache.entries.get(&entry.0).is_some()); @@ -757,7 +843,7 @@ mod tests { usage_counter: AtomicU64, ) -> Arc { Arc::new(LoadedProgram { - program: LoadedProgramType::BuiltIn(BuiltInProgram::default()), + program: LoadedProgramType::TestLoaded, account_size: 0, deployment_slot, effective_slot, diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 0b87b057521944..dd079e5d0096f5 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -36,6 +36,7 @@ num-traits = { workspace = true } num_cpus = { workspace = true } once_cell = { workspace = true } ouroboros = { workspace = true } +percentage = { workspace = true } rand = { workspace = true } rayon = { workspace = true } regex = { workspace = true } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c0e59ff9815e23..4daf6430502267 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -83,6 +83,7 @@ use { dashmap::{DashMap, DashSet}, itertools::Itertools, log::*, + percentage::Percentage, rayon::{ iter::{IntoParallelIterator, IntoParallelRefIterator, ParallelIterator}, ThreadPool, ThreadPoolBuilder, @@ -4641,10 +4642,11 @@ impl Bank { execution_time.stop(); + const PRUNE_CACHE_TO_PERCENTAGE: u8 = 90; self.loaded_programs_cache .write() .unwrap() - .sort_and_unload(None); + .sort_and_evict(Percentage::from(PRUNE_CACHE_TO_PERCENTAGE)); debug!( "check: {}us load: {}us execute: {}us txs_len={}", From 04c20579fc40ae2d47907acdd6104972670b2917 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Sat, 8 Apr 2023 07:26:38 +0530 Subject: [PATCH 3/6] unit tests, and address review comments --- program-runtime/src/loaded_programs.rs | 316 +++++++++++++++++++++++-- programs/sbf/Cargo.lock | 2 + runtime/src/bank.rs | 4 +- 3 files changed, 295 insertions(+), 27 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index b1f2927d9f1714..d9fd9a064bbe57 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -14,6 +14,7 @@ use { pubkey::Pubkey, saturating_add_assign, }, std::{ + cmp, collections::HashMap, fmt::{Debug, Formatter}, sync::{ @@ -24,8 +25,8 @@ use { }; const MAX_LOADED_ENTRY_COUNT: usize = 256; -const MAX_UNLOADED_ENTRY_COUNT: usize = 512; -const MAX_TOMBSTONE_COUNT: usize = 512; +const MAX_UNLOADED_ENTRY_COUNT: usize = 1024; +const MAX_TOMBSTONE_COUNT: usize = 1024; /// Relationship between two fork IDs #[derive(Copy, Clone, PartialEq)] @@ -339,6 +340,7 @@ impl LoadedPrograms { }); self.remove_expired_entries(new_root); + self.remove_programs_with_no_entries(); } /// Extracts a subset of the programs relevant to a transaction batch @@ -428,15 +430,25 @@ impl LoadedPrograms { let num_to_unload = num_loaded.saturating_sub(shrink_to.apply_to(MAX_LOADED_ENTRY_COUNT)); self.unload_program_entries(sorted_candidates.iter().take(num_to_unload)); - let num_unloaded_to_evict = - num_unloaded.saturating_sub(shrink_to.apply_to(MAX_UNLOADED_ENTRY_COUNT)); - let (sorted_candidates, unloaded) = sorted_candidates.split_at(num_loaded); - self.remove_program_entries(unloaded.iter().take(num_unloaded_to_evict)); + let num_unloaded_to_evict = num_unloaded + .saturating_add(num_to_unload) + .saturating_sub(shrink_to.apply_to(MAX_UNLOADED_ENTRY_COUNT)); + let (newly_unloaded_programs, sorted_candidates) = sorted_candidates.split_at(num_loaded); + let num_old_unloaded_to_evict = cmp::min(sorted_candidates.len(), num_unloaded_to_evict); + self.remove_program_entries(sorted_candidates.iter().take(num_old_unloaded_to_evict)); + + let num_newly_unloaded_to_evict = + num_unloaded_to_evict.saturating_sub(sorted_candidates.len()); + self.remove_program_entries( + newly_unloaded_programs + .iter() + .take(num_newly_unloaded_to_evict), + ); let num_tombstones_to_evict = num_tombstones.saturating_sub(shrink_to.apply_to(MAX_TOMBSTONE_COUNT)); - let (_sorted_candidates, tombstones) = sorted_candidates.split_at(num_unloaded); - self.remove_program_entries(tombstones.iter().take(num_tombstones_to_evict)); + let (_, sorted_candidates) = sorted_candidates.split_at(num_unloaded); + self.remove_program_entries(sorted_candidates.iter().take(num_tombstones_to_evict)); self.remove_programs_with_no_entries(); } @@ -471,7 +483,6 @@ impl LoadedPrograms { .unwrap_or(true) }); } - self.remove_programs_with_no_entries(); } fn unload_program_entries<'a>( @@ -533,22 +544,40 @@ mod tests { ) } + fn insert_unloaded_program( + cache: &mut LoadedPrograms, + key: Pubkey, + slot: Slot, + ) -> Arc { + let unloaded = Arc::new(LoadedProgram { + program: LoadedProgramType::Unloaded, + account_size: 0, + deployment_slot: slot, + effective_slot: slot.saturating_add(1), + maybe_expiration_slot: None, + usage_counter: AtomicU64::default(), + }); + cache.replenish(key, unloaded).1 + } + + fn num_matching_entries

(cache: &LoadedPrograms, predicate: P) -> usize + where + P: Fn(&LoadedProgramType) -> bool, + { + cache + .entries + .values() + .map(|programs| { + programs + .iter() + .filter(|program| predicate(&program.program)) + .count() + }) + .sum() + } + #[test] fn test_eviction() { - // Fork graph created for the test - // 0 - // / \ - // 10 5 - // | | - // 20 11 - // | | \ - // 22 15 25 - // | | - // 16 27 - let mut fork_graph = TestForkGraphSpecific::default(); - fork_graph.insert_fork(&[0, 10, 20, 22]); - fork_graph.insert_fork(&[0, 5, 11, 15, 16]); - fork_graph.insert_fork(&[0, 5, 11, 25, 27]); let mut programs = vec![]; let mut num_total_programs: usize = 0; @@ -556,7 +585,7 @@ mod tests { let program1 = Pubkey::new_unique(); let program1_deployment_slots = vec![0, 10, 20]; - let program1_usage_counters = vec![1, 5, 25]; + let program1_usage_counters = vec![4, 5, 25]; program1_deployment_slots .iter() .enumerate() @@ -574,9 +603,17 @@ mod tests { programs.push((program1, *deployment_slot, usage_counter)); }); + for slot in 21..31 { + set_tombstone(&mut cache, program1, slot); + } + + for slot in 31..41 { + insert_unloaded_program(&mut cache, program1, slot); + } + let program2 = Pubkey::new_unique(); let program2_deployment_slots = vec![5, 11]; - let program2_usage_counters = vec![0, 10]; + let program2_usage_counters = vec![0, 2]; program2_deployment_slots .iter() .enumerate() @@ -594,6 +631,14 @@ mod tests { programs.push((program2, *deployment_slot, usage_counter)); }); + for slot in 21..31 { + set_tombstone(&mut cache, program2, slot); + } + + for slot in 31..41 { + insert_unloaded_program(&mut cache, program2, slot); + } + let program3 = Pubkey::new_unique(); let program3_deployment_slots = vec![0, 5, 15]; let program3_usage_counters = vec![100, 3, 20]; @@ -614,8 +659,39 @@ mod tests { programs.push((program3, *deployment_slot, usage_counter)); }); + for slot in 21..31 { + set_tombstone(&mut cache, program3, slot); + } + + 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 + // * 20 unloaded entries + // * 20 tombstones cache.sort_and_evict(Percentage::from(2)); // Check that every program is still in the cache. programs.iter().for_each(|entry| { @@ -637,6 +713,98 @@ mod tests { let expected = programs.get(index).expect("Missing program"); assert!(unloaded.contains(&(expected.0, expected.2))); } + + 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, 20); + assert_eq!(num_tombstones, 20); + } + + #[test] + fn test_eviction_unload_underflow() { + // Test: Eviction of unloaded programs requires eviction of newly unloaded programs. + // 1. Load 26 programs + // 2. Insert 1 unloaded program + // Eviction will unload 21 programs. + // 2 unloaded programs need to be evicted. So 1 old and 1 new unloaded program will be evicted. + + let mut cache = LoadedPrograms::default(); + + let program1 = Pubkey::new_unique(); + let num_total_programs = 26; + (0..num_total_programs).for_each(|i| { + cache.replenish( + program1, + new_test_loaded_program_with_usage(i, i + 2, AtomicU64::new(i)), + ); + }); + + let program2 = Pubkey::new_unique(); + insert_unloaded_program(&mut cache, program2, 26); + + 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, 26); + assert_eq!(num_unloaded, 1); + assert_eq!(num_tombstones, 0); + + // Test that program2 exists in the cache. It'll get removed after eviction. + assert!(cache.entries.get(&program2).is_some()); + + // Evicting to 2% should update cache with + // * 5 active entries + // * 20 unloaded entries + // * 0 tombstones + cache.sort_and_evict(Percentage::from(2)); + + 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, 20); + assert_eq!(num_tombstones, 0); + + // Test that program2 has been removed after eviction. + assert!(cache.entries.get(&program2).is_none()); } #[test] @@ -1112,4 +1280,102 @@ mod tests { // program3 was deployed on slot 25, which has been pruned assert!(missing.contains(&program3)); } + + #[test] + fn test_prune_expired() { + let mut cache = LoadedPrograms::default(); + + // Fork graph created for the test + // 0 + // / \ + // 10 5 + // | | + // 20 11 + // | | \ + // 22 15 25 + // | | + // 16 27 + // | + // 19 + // | + // 23 + + let mut fork_graph = TestForkGraphSpecific::default(); + fork_graph.insert_fork(&[0, 10, 20, 22]); + fork_graph.insert_fork(&[0, 5, 11, 15, 16, 19, 21, 23]); + fork_graph.insert_fork(&[0, 5, 11, 25, 27]); + + let program1 = Pubkey::new_unique(); + assert!(!cache.replenish(program1, new_test_loaded_program(10, 11)).0); + assert!(!cache.replenish(program1, new_test_loaded_program(20, 21)).0); + + let program2 = Pubkey::new_unique(); + assert!(!cache.replenish(program2, new_test_loaded_program(5, 6)).0); + assert!(!cache.replenish(program2, new_test_loaded_program(11, 12)).0); + + let program3 = Pubkey::new_unique(); + assert!(!cache.replenish(program3, new_test_loaded_program(25, 26)).0); + + // The following is a special case, where there's an expiration slot + let test_program = Arc::new(LoadedProgram { + program: LoadedProgramType::DelayVisibility, + account_size: 0, + deployment_slot: 11, + effective_slot: 11, + maybe_expiration_slot: Some(15), + usage_counter: AtomicU64::default(), + }); + assert!(!cache.replenish(program1, test_program).0); + + // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 + let working_slot = TestWorkingSlot::new(12, &[0, 5, 11, 12, 15, 16, 18, 19, 21, 23]); + let (found, missing) = cache.extract( + &working_slot, + vec![program1, program2, program3].into_iter(), + ); + + // Program1 deployed at slot 11 should not be expired yet + assert!(match_slot(&found, &program1, 11)); + assert!(match_slot(&found, &program2, 11)); + + assert!(missing.contains(&program3)); + + // 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 working_slot = TestWorkingSlot::new(15, &[0, 5, 11, 15, 16, 18, 19, 21, 23]); + let (found, missing) = cache.extract( + &working_slot, + vec![program1, program2, program3].into_iter(), + ); + + assert!(match_slot(&found, &program2, 11)); + + assert!(missing.contains(&program1)); + assert!(missing.contains(&program3)); + + // Test that the program still exists in the cache, even though it is expired. + assert_eq!( + cache + .entries + .get(&program1) + .expect("Didn't find program1") + .len(), + 3 + ); + + // Nww root 5 should not evict the expired entry for program1 + cache.prune(&fork_graph, 5); + assert_eq!( + cache + .entries + .get(&program1) + .expect("Didn't find program1") + .len(), + 1 + ); + + // Nww root 15 should evict the expired entry for program1 + cache.prune(&fork_graph, 15); + assert!(cache.entries.get(&program1).is_none()); + } } diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index d7e7355b5c2e5e..472bf2190e9ddd 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5312,6 +5312,7 @@ dependencies = [ "log", "num-derive", "num-traits", + "percentage", "rand 0.7.3", "rustc_version 0.4.0", "serde", @@ -5560,6 +5561,7 @@ dependencies = [ "num_cpus", "once_cell", "ouroboros", + "percentage", "rand 0.7.3", "rayon", "regex", diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4daf6430502267..ffa605091df734 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4642,11 +4642,11 @@ impl Bank { execution_time.stop(); - const PRUNE_CACHE_TO_PERCENTAGE: u8 = 90; + const EVICT_CACHE_TO_PERCENTAGE: u8 = 90; self.loaded_programs_cache .write() .unwrap() - .sort_and_evict(Percentage::from(PRUNE_CACHE_TO_PERCENTAGE)); + .sort_and_evict(Percentage::from(EVICT_CACHE_TO_PERCENTAGE)); debug!( "check: {}us load: {}us execute: {}us txs_len={}", From 6c196a3121279c09f80e8c6e95a7ce9376fcbfe7 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Sat, 8 Apr 2023 07:42:39 +0530 Subject: [PATCH 4/6] fix clippy --- program-runtime/src/loaded_programs.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index d9fd9a064bbe57..4219bbcf0dc174 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -491,10 +491,9 @@ impl LoadedPrograms { ) { for (id, program) in remove { if let Some(entries) = self.entries.get_mut(id) { - entries - .iter_mut() - .find(|entry| entry == &program) - .map(|candidate| *candidate = Arc::new(candidate.to_unloaded())); + if let Some(candidate) = entries.iter_mut().find(|entry| entry == &program) { + *candidate = Arc::new(candidate.to_unloaded()); + } } } } From 94d9c9b2920b9a0e2f5ad9b488c6e18b250ed891 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 10 Apr 2023 15:45:00 +0530 Subject: [PATCH 5/6] address review comments --- program-runtime/src/loaded_programs.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 4219bbcf0dc174..7d134515e6a2a4 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -295,6 +295,11 @@ impl LoadedPrograms { { if matches!(existing.program, LoadedProgramType::Unloaded) { // The unloaded program is getting reloaded + // Copy over the usage counter to the new entry + entry.usage_counter.store( + existing.usage_counter.load(Ordering::Relaxed), + Ordering::Relaxed, + ); second_level.swap_remove(entry_index); } else { return (true, existing.clone()); @@ -418,12 +423,11 @@ impl LoadedPrograms { .unzip(); for order in ordering { - if order == 0 { - num_loaded = num_loaded.saturating_add(1); - } else if order == 1 { - num_unloaded = num_unloaded.saturating_add(1); - } else if order == 2 { - num_tombstones = num_tombstones.saturating_add(1); + match order { + 0 => num_loaded = num_loaded.saturating_add(1), + 1 => num_unloaded = num_unloaded.saturating_add(1), + 2 => num_tombstones = num_tombstones.saturating_add(1), + _ => unreachable!(), } } @@ -548,14 +552,8 @@ mod tests { key: Pubkey, slot: Slot, ) -> Arc { - let unloaded = Arc::new(LoadedProgram { - program: LoadedProgramType::Unloaded, - account_size: 0, - deployment_slot: slot, - effective_slot: slot.saturating_add(1), - maybe_expiration_slot: None, - usage_counter: AtomicU64::default(), - }); + let unloaded = + Arc::new(new_test_loaded_program(slot, slot.saturating_add(1)).to_unloaded()); cache.replenish(key, unloaded).1 } From 04e79b9160d7a1ecbee6b51d73b3f5cebe0894bb Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 10 Apr 2023 21:06:55 +0530 Subject: [PATCH 6/6] new unit test and fixes to comments --- program-runtime/src/loaded_programs.rs | 57 +++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 7d134515e6a2a4..ae5000936b3128 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -65,6 +65,7 @@ pub enum LoadedProgramType { FailedVerification, Closed, DelayVisibility, + /// Successfully verified but not currently compiled, used to track usage statistics when a compiled program is evicted from memory. Unloaded, LegacyV0(VerifiedExecutable>), LegacyV1(VerifiedExecutable>), @@ -804,6 +805,58 @@ mod tests { assert!(cache.entries.get(&program2).is_none()); } + #[test] + fn test_usage_count_of_unloaded_program() { + let mut cache = LoadedPrograms::default(); + + let program = Pubkey::new_unique(); + let num_total_programs = 6; + (0..num_total_programs).for_each(|i| { + cache.replenish( + program, + new_test_loaded_program_with_usage(i, i + 2, AtomicU64::new(i + 10)), + ); + }); + + // This will unload the program deployed at slot 0, with usage count = 10 + cache.sort_and_evict(Percentage::from(2)); + + let num_unloaded = num_matching_entries(&cache, |program_type| { + matches!(program_type, LoadedProgramType::Unloaded) + }); + assert_eq!(num_unloaded, 1); + + cache.entries.values().for_each(|programs| { + programs.iter().for_each(|program| { + if matches!(program.program, LoadedProgramType::Unloaded) { + // Test that the usage counter is retained for the unloaded program + assert_eq!(program.usage_counter.load(Ordering::Relaxed), 10); + assert_eq!(program.deployment_slot, 0); + assert_eq!(program.effective_slot, 2); + } + }) + }); + + // Replenish the program that was just unloaded. Use 0 as the usage counter. This should be + // updated with the usage counter from the unloaded program. + cache.replenish( + program, + new_test_loaded_program_with_usage(0, 2, AtomicU64::new(0)), + ); + + cache.entries.values().for_each(|programs| { + programs.iter().for_each(|program| { + if matches!(program.program, LoadedProgramType::Unloaded) + && program.deployment_slot == 0 + && program.effective_slot == 2 + { + // Test that the usage counter was correctly updated. + assert_eq!(program.usage_counter.load(Ordering::Relaxed), 10); + } + }) + }); + } + #[test] fn test_tombstone() { let tombstone = LoadedProgram::new_tombstone(0, LoadedProgramType::FailedVerification); @@ -1360,7 +1413,7 @@ mod tests { 3 ); - // Nww root 5 should not evict the expired entry for program1 + // New root 5 should not evict the expired entry for program1 cache.prune(&fork_graph, 5); assert_eq!( cache @@ -1371,7 +1424,7 @@ mod tests { 1 ); - // Nww root 15 should evict the expired entry for program1 + // New root 15 should evict the expired entry for program1 cache.prune(&fork_graph, 15); assert!(cache.entries.get(&program1).is_none()); }