From cc1227911b72a2654f3bbab22ec2692185915788 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Thu, 25 Apr 2024 14:54:52 +0900 Subject: [PATCH 1/3] Reduce ProgramCache write-lock contention --- program-runtime/src/loaded_programs.rs | 7 +++++++ runtime/src/bank.rs | 2 +- svm/src/transaction_processor.rs | 23 +++++++++++++++-------- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 3b19342f7cfc67..47a5315c7f79f9 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -638,6 +638,7 @@ pub struct ProgramCacheForTxBatch { /// The epoch of the last rerooting pub latest_root_epoch: Epoch, pub hit_max_limit: bool, + pub loaded_missing: bool, } impl ProgramCacheForTxBatch { @@ -654,6 +655,7 @@ impl ProgramCacheForTxBatch { upcoming_environments, latest_root_epoch, hit_max_limit: false, + loaded_missing: false, } } @@ -669,6 +671,7 @@ impl ProgramCacheForTxBatch { upcoming_environments: cache.get_upcoming_environments_for_epoch(epoch), latest_root_epoch: cache.latest_root_epoch, hit_max_limit: false, + loaded_missing: false, } } @@ -725,6 +728,10 @@ impl ProgramCacheForTxBatch { self.replenish(*key, entry.clone()); }) } + + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } } pub enum ProgramCacheMatchCriteria { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index de7dcf78b4c625..05dd92bc35a52a 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4162,7 +4162,7 @@ impl Bank { programs_modified_by_tx, } = execution_result { - if details.status.is_ok() { + if details.status.is_ok() && !programs_modified_by_tx.is_empty() { let mut cache = self.transaction_processor.program_cache.write().unwrap(); cache.merge(programs_modified_by_tx); } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 9b7cc4b6956bbc..669b3ed4144b16 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -292,14 +292,20 @@ impl TransactionBatchProcessor { execution_time.stop(); - const SHRINK_LOADED_PROGRAMS_TO_PERCENTAGE: u8 = 90; - self.program_cache - .write() - .unwrap() - .evict_using_2s_random_selection( - Percentage::from(SHRINK_LOADED_PROGRAMS_TO_PERCENTAGE), - self.slot, - ); + // Skip eviction when there's no chance this particular tx batch has increased the size of + // ProgramCache entries. Note that this flag is deliberately defined, so that there's still + // at least one other batch, which will evict the program cache, even after the occurrences + // of cooperative loading. + if programs_loaded_for_tx_batch.borrow().loaded_missing { + const SHRINK_LOADED_PROGRAMS_TO_PERCENTAGE: u8 = 90; + self.program_cache + .write() + .unwrap() + .evict_using_2s_random_selection( + Percentage::from(SHRINK_LOADED_PROGRAMS_TO_PERCENTAGE), + self.slot, + ); + } debug!( "load: {}us execute: {}us txs_len={}", @@ -395,6 +401,7 @@ impl TransactionBatchProcessor { } // Submit our last completed loading task. if let Some((key, program)) = program_to_store.take() { + loaded_programs_for_txs.as_mut().unwrap().loaded_missing = true; if program_cache.finish_cooperative_loading_task(self.slot, key, program) && limit_to_load_programs { From d50c11cf1791d27e446459577728e2db4ffd1e30 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Thu, 25 Apr 2024 17:42:33 +0900 Subject: [PATCH 2/3] Also consider programs modified by tx --- program-runtime/src/loaded_programs.rs | 4 ++++ svm/src/transaction_processor.rs | 10 ++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 47a5315c7f79f9..f54829b69837c0 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -639,6 +639,7 @@ pub struct ProgramCacheForTxBatch { pub latest_root_epoch: Epoch, pub hit_max_limit: bool, pub loaded_missing: bool, + pub merged_modified: bool, } impl ProgramCacheForTxBatch { @@ -656,6 +657,7 @@ impl ProgramCacheForTxBatch { latest_root_epoch, hit_max_limit: false, loaded_missing: false, + merged_modified: false, } } @@ -672,6 +674,7 @@ impl ProgramCacheForTxBatch { latest_root_epoch: cache.latest_root_epoch, hit_max_limit: false, loaded_missing: false, + merged_modified: false, } } @@ -725,6 +728,7 @@ impl ProgramCacheForTxBatch { pub fn merge(&mut self, other: &Self) { other.entries.iter().for_each(|(key, entry)| { + self.merged_modified = true; self.replenish(*key, entry.clone()); }) } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 669b3ed4144b16..4fb54e5a1f1713 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -293,10 +293,12 @@ impl TransactionBatchProcessor { execution_time.stop(); // Skip eviction when there's no chance this particular tx batch has increased the size of - // ProgramCache entries. Note that this flag is deliberately defined, so that there's still - // at least one other batch, which will evict the program cache, even after the occurrences - // of cooperative loading. - if programs_loaded_for_tx_batch.borrow().loaded_missing { + // ProgramCache entries. Note that loaded_missing is deliberately defined, so that there's + // still at least one other batch, which will evict the program cache, even after the + // occurrences of cooperative loading. + if programs_loaded_for_tx_batch.borrow().loaded_missing + || programs_loaded_for_tx_batch.borrow().merged_modified + { const SHRINK_LOADED_PROGRAMS_TO_PERCENTAGE: u8 = 90; self.program_cache .write() From cce3075a294e7360f22b2262112dff7eee1940b7 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 26 Apr 2024 09:25:10 +0900 Subject: [PATCH 3/3] Memoize program cache write lock --- runtime/src/bank.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 05dd92bc35a52a..17ee43f41cc83e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4156,6 +4156,7 @@ impl Bank { let mut store_executors_which_were_deployed_time = Measure::start("store_executors_which_were_deployed_time"); + let mut cache = None; for execution_result in &execution_results { if let TransactionExecutionResult::Executed { details, @@ -4163,11 +4164,15 @@ impl Bank { } = execution_result { if details.status.is_ok() && !programs_modified_by_tx.is_empty() { - let mut cache = self.transaction_processor.program_cache.write().unwrap(); - cache.merge(programs_modified_by_tx); + cache + .get_or_insert_with(|| { + self.transaction_processor.program_cache.write().unwrap() + }) + .merge(programs_modified_by_tx); } } } + drop(cache); store_executors_which_were_deployed_time.stop(); saturating_add_assign!( timings.execute_accessories.update_executors_us,