From 29ad081555967166bc0881b5eaed97925a609fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Thu, 4 Nov 2021 09:48:34 +0100 Subject: [PATCH] Stop caching sysvars, instead load them ahead of time. (#21108) --- programs/bpf/tests/programs.rs | 4 +- programs/bpf_loader/src/syscalls.rs | 24 +++---- programs/stake/src/stake_instruction.rs | 14 ++-- runtime/src/bank.rs | 30 ++++++++- runtime/src/message_processor.rs | 87 +++++++------------------ sdk/program/src/sysvar/mod.rs | 30 +++++---- sdk/src/process_instruction.rs | 42 +++++------- 7 files changed, 105 insertions(+), 126 deletions(-) diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 39a10212d..b5fb410e5 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -1400,9 +1400,9 @@ fn assert_instruction_count() { ("solana_bpf_rust_noop", 480), ("solana_bpf_rust_param_passing", 146), ("solana_bpf_rust_rand", 487), - ("solana_bpf_rust_sanity", 8406), + ("solana_bpf_rust_sanity", 8454), ("solana_bpf_rust_secp256k1_recover", 25216), - ("solana_bpf_rust_sha", 30704), + ("solana_bpf_rust_sha", 30692), ]); } diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 0144debd2..195bf863d 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -3284,10 +3284,8 @@ mod tests { let mut invoke_context = MockInvokeContext::new(&Pubkey::default(), vec![]); let mut data = vec![]; bincode::serialize_into(&mut data, &src_clock).unwrap(); - invoke_context - .get_sysvars() - .borrow_mut() - .push((sysvar::clock::id(), Some(Rc::new(data)))); + let sysvars = &[(sysvar::clock::id(), data)]; + invoke_context.sysvars = sysvars; let mut syscall = SyscallGetClockSysvar { invoke_context: Rc::new(RefCell::new(&mut invoke_context)), @@ -3330,10 +3328,8 @@ mod tests { let mut invoke_context = MockInvokeContext::new(&Pubkey::default(), vec![]); let mut data = vec![]; bincode::serialize_into(&mut data, &src_epochschedule).unwrap(); - invoke_context - .get_sysvars() - .borrow_mut() - .push((sysvar::epoch_schedule::id(), Some(Rc::new(data)))); + let sysvars = &[(sysvar::epoch_schedule::id(), data)]; + invoke_context.sysvars = sysvars; let mut syscall = SyscallGetEpochScheduleSysvar { invoke_context: Rc::new(RefCell::new(&mut invoke_context)), @@ -3383,10 +3379,8 @@ mod tests { let mut invoke_context = MockInvokeContext::new(&Pubkey::default(), vec![]); let mut data = vec![]; bincode::serialize_into(&mut data, &src_fees).unwrap(); - invoke_context - .get_sysvars() - .borrow_mut() - .push((sysvar::fees::id(), Some(Rc::new(data)))); + let sysvars = &[(sysvar::fees::id(), data)]; + invoke_context.sysvars = sysvars; let mut syscall = SyscallGetFeesSysvar { invoke_context: Rc::new(RefCell::new(&mut invoke_context)), @@ -3427,10 +3421,8 @@ mod tests { let mut invoke_context = MockInvokeContext::new(&Pubkey::default(), vec![]); let mut data = vec![]; bincode::serialize_into(&mut data, &src_rent).unwrap(); - invoke_context - .get_sysvars() - .borrow_mut() - .push((sysvar::rent::id(), Some(Rc::new(data)))); + let sysvars = &[(sysvar::rent::id(), data)]; + invoke_context.sysvars = sysvars; let mut syscall = SyscallGetRentSysvar { invoke_context: Rc::new(RefCell::new(&mut invoke_context)), diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index f962a69eb..3041dfc4b 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -340,7 +340,7 @@ mod tests { }, sysvar::{stake_history::StakeHistory, Sysvar}, }; - use std::{cell::RefCell, rc::Rc, str::FromStr}; + use std::{cell::RefCell, str::FromStr}; fn create_default_account() -> RefCell { RefCell::new(AccountSharedData::default()) @@ -444,10 +444,8 @@ mod tests { ); let mut data = Vec::with_capacity(sysvar::clock::Clock::size_of()); bincode::serialize_into(&mut data, &sysvar::clock::Clock::default()).unwrap(); - invoke_context - .get_sysvars() - .borrow_mut() - .push((sysvar::clock::id(), Some(Rc::new(data)))); + let sysvars = &[(sysvar::clock::id(), data)]; + invoke_context.sysvars = sysvars; super::process_instruction(1, &instruction.data, &mut invoke_context) } } @@ -1102,10 +1100,8 @@ mod tests { MockInvokeContext::new(&id(), create_keyed_accounts_unified(&keyed_accounts)); let mut data = Vec::with_capacity(sysvar::clock::Clock::size_of()); bincode::serialize_into(&mut data, &sysvar::clock::Clock::default()).unwrap(); - invoke_context - .get_sysvars() - .borrow_mut() - .push((sysvar::clock::id(), Some(Rc::new(data)))); + let sysvars = &[(sysvar::clock::id(), data)]; + invoke_context.sysvars = sysvars; assert_eq!( super::process_instruction( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 6a59cae7c..1ba9d5eef 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1006,6 +1006,8 @@ pub struct Bank { vote_only_bank: bool, pub cost_tracker: RwLock, + + sysvar_cache: RwLock)>>, } impl Default for BlockhashQueue { @@ -1145,6 +1147,7 @@ impl Bank { freeze_started: AtomicBool::default(), vote_only_bank: false, cost_tracker: RwLock::::default(), + sysvar_cache: RwLock::new(Vec::new()), } } @@ -1254,6 +1257,7 @@ impl Bank { bank.update_rent(); bank.update_epoch_schedule(); bank.update_recent_blockhashes(); + bank.fill_sysvar_cache(); bank } @@ -1402,6 +1406,7 @@ impl Bank { )), freeze_started: AtomicBool::new(false), cost_tracker: RwLock::new(CostTracker::default()), + sysvar_cache: RwLock::new(Vec::new()), }; datapoint_info!( @@ -1458,6 +1463,7 @@ impl Bank { new.update_stake_history(Some(parent_epoch)); new.update_clock(Some(parent_epoch)); new.update_fees(); + new.fill_sysvar_cache(); return new; } @@ -1473,6 +1479,7 @@ impl Bank { new.update_stake_history(Some(parent_epoch)); new.update_clock(Some(parent_epoch)); new.update_fees(); + new.fill_sysvar_cache(); new } @@ -1589,6 +1596,7 @@ impl Bank { freeze_started: AtomicBool::new(fields.hash != Hash::default()), vote_only_bank: false, cost_tracker: RwLock::new(CostTracker::default()), + sysvar_cache: RwLock::new(Vec::new()), }; bank.finish_init( genesis_config, @@ -1767,6 +1775,14 @@ impl Bank { } self.store_account_and_update_capitalization(pubkey, &new_account); + + // Update the entry in the cache + let mut sysvar_cache = self.sysvar_cache.write().unwrap(); + if let Some(position) = sysvar_cache.iter().position(|(id, _data)| id == pubkey) { + sysvar_cache[position].1 = new_account.data().to_vec(); + } else { + sysvar_cache.push((*pubkey, new_account.data().to_vec())); + } } fn inherit_specially_retained_account_fields( @@ -1904,6 +1920,17 @@ impl Bank { }); } + fn fill_sysvar_cache(&mut self) { + let mut sysvar_cache = self.sysvar_cache.write().unwrap(); + for id in sysvar::ALL_IDS.iter() { + if !sysvar_cache.iter().any(|(key, _data)| key == id) { + if let Some(account) = self.get_account_with_fixed_root(id) { + sysvar_cache.push((*id, account.data().to_vec())); + } + } + } + } + pub fn get_slot_history(&self) -> SlotHistory { from_account(&self.get_account(&sysvar::slot_history::id()).unwrap()).unwrap() } @@ -3858,8 +3885,7 @@ impl Bank { compute_budget, compute_meter, &mut timings.details, - self.rc.accounts.clone(), - &self.ancestors, + &*self.sysvar_cache.read().unwrap(), blockhash, lamports_per_signature, ); diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 71e5d9182..83f2bc552 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -1,6 +1,6 @@ use crate::{ - accounts::Accounts, ancestors::Ancestors, instruction_recorder::InstructionRecorder, - log_collector::LogCollector, rent_collector::RentCollector, + instruction_recorder::InstructionRecorder, log_collector::LogCollector, + rent_collector::RentCollector, }; use log::*; use serde::{Deserialize, Serialize}; @@ -80,6 +80,7 @@ pub struct ThisInvokeContext<'a> { pre_accounts: Vec, accounts: &'a [(Pubkey, Rc>)], programs: &'a [(Pubkey, ProcessInstructionWithContext)], + sysvars: &'a [(Pubkey, Vec)], logger: Rc>, compute_budget: ComputeBudget, compute_meter: Rc>, @@ -87,10 +88,6 @@ pub struct ThisInvokeContext<'a> { instruction_recorders: Option<&'a [InstructionRecorder]>, feature_set: Arc, pub timings: ExecuteDetailsTimings, - account_db: Arc, - ancestors: Option<&'a Ancestors>, - #[allow(clippy::type_complexity)] - sysvars: RefCell>>)>>, blockhash: Hash, lamports_per_signature: u64, return_data: (Pubkey, Vec), @@ -101,14 +98,13 @@ impl<'a> ThisInvokeContext<'a> { rent: Rent, accounts: &'a [(Pubkey, Rc>)], programs: &'a [(Pubkey, ProcessInstructionWithContext)], + sysvars: &'a [(Pubkey, Vec)], log_collector: Option>, compute_budget: ComputeBudget, compute_meter: Rc>, executors: Rc>, instruction_recorders: Option<&'a [InstructionRecorder]>, feature_set: Arc, - account_db: Arc, - ancestors: Option<&'a Ancestors>, blockhash: Hash, lamports_per_signature: u64, ) -> Self { @@ -119,6 +115,7 @@ impl<'a> ThisInvokeContext<'a> { pre_accounts: Vec::new(), accounts, programs, + sysvars, logger: ThisLogger::new_ref(log_collector), compute_budget, compute_meter, @@ -126,32 +123,29 @@ impl<'a> ThisInvokeContext<'a> { instruction_recorders, feature_set, timings: ExecuteDetailsTimings::default(), - account_db, - ancestors, - sysvars: RefCell::new(Vec::new()), blockhash, lamports_per_signature, return_data: (Pubkey::default(), Vec::new()), } } - pub fn new_mock_with_features( + pub fn new_mock_with_sysvars_and_features( accounts: &'a [(Pubkey, Rc>)], programs: &'a [(Pubkey, ProcessInstructionWithContext)], + sysvars: &'a [(Pubkey, Vec)], feature_set: Arc, ) -> Self { Self::new( Rent::default(), accounts, programs, + sysvars, None, ComputeBudget::default(), ThisComputeMeter::new_ref(std::i64::MAX as u64), Rc::new(RefCell::new(Executors::default())), None, feature_set, - Arc::new(Accounts::default_for_tests()), - None, Hash::default(), 0, ) @@ -161,7 +155,12 @@ impl<'a> ThisInvokeContext<'a> { accounts: &'a [(Pubkey, Rc>)], programs: &'a [(Pubkey, ProcessInstructionWithContext)], ) -> Self { - Self::new_mock_with_features(accounts, programs, Arc::new(FeatureSet::all_enabled())) + Self::new_mock_with_sysvars_and_features( + accounts, + programs, + &[], + Arc::new(FeatureSet::all_enabled()), + ) } } impl<'a> InvokeContext for ThisInvokeContext<'a> { @@ -452,31 +451,8 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { self.timings.execute_us += execute_us; self.timings.deserialize_us += deserialize_us; } - #[allow(clippy::type_complexity)] - fn get_sysvars(&self) -> &RefCell>>)>> { - &self.sysvars - } - fn get_sysvar_data(&self, id: &Pubkey) -> Option>> { - if let Ok(mut sysvars) = self.sysvars.try_borrow_mut() { - // Try share from cache - let mut result = sysvars - .iter() - .find_map(|(key, sysvar)| if id == key { sysvar.clone() } else { None }); - if result.is_none() { - if let Some(ancestors) = self.ancestors { - // Load it - result = self - .account_db - .load_with_fixed_root(ancestors, id) - .map(|(account, _)| Rc::new(account.data().to_vec())); - // Cache it - sysvars.push((*id, result.clone())); - } - } - result - } else { - None - } + fn get_sysvars(&self) -> &[(Pubkey, Vec)] { + self.sysvars } fn get_compute_budget(&self) -> &ComputeBudget { &self.compute_budget @@ -604,7 +580,6 @@ impl MessageProcessor { /// the call does not violate the bank's accounting rules. /// The accounts are committed back to the bank only if every instruction succeeds. #[allow(clippy::too_many_arguments)] - #[allow(clippy::type_complexity)] pub fn process_message( instruction_processor: &InstructionProcessor, message: &Message, @@ -618,8 +593,7 @@ impl MessageProcessor { compute_budget: ComputeBudget, compute_meter: Rc>, timings: &mut ExecuteDetailsTimings, - account_db: Arc, - ancestors: &Ancestors, + sysvars: &[(Pubkey, Vec)], blockhash: Hash, lamports_per_signature: u64, ) -> Result<(), TransactionError> { @@ -627,14 +601,13 @@ impl MessageProcessor { rent_collector.rent, accounts, instruction_processor.programs(), + sysvars, log_collector, compute_budget, compute_meter, executors, instruction_recorders, feature_set, - account_db, - Some(ancestors), blockhash, lamports_per_signature, ); @@ -979,7 +952,6 @@ mod tests { let program_indices = vec![vec![2]]; let executors = Rc::new(RefCell::new(Executors::default())); - let ancestors = Ancestors::default(); let account_metas = vec![ AccountMeta::new(accounts[0].0, true), @@ -1007,8 +979,7 @@ mod tests { ComputeBudget::new(), ThisComputeMeter::new_ref(std::i64::MAX as u64), &mut ExecuteDetailsTimings::default(), - Arc::new(Accounts::default_for_tests()), - &ancestors, + &[], Hash::default(), 0, ); @@ -1038,8 +1009,7 @@ mod tests { ComputeBudget::new(), ThisComputeMeter::new_ref(std::i64::MAX as u64), &mut ExecuteDetailsTimings::default(), - Arc::new(Accounts::default_for_tests()), - &ancestors, + &[], Hash::default(), 0, ); @@ -1073,8 +1043,7 @@ mod tests { ComputeBudget::new(), ThisComputeMeter::new_ref(std::i64::MAX as u64), &mut ExecuteDetailsTimings::default(), - Arc::new(Accounts::default_for_tests()), - &ancestors, + &[], Hash::default(), 0, ); @@ -1188,7 +1157,6 @@ mod tests { let program_indices = vec![vec![2]]; let executors = Rc::new(RefCell::new(Executors::default())); - let ancestors = Ancestors::default(); let account_metas = vec![ AccountMeta::new(accounts[0].0, true), @@ -1218,8 +1186,7 @@ mod tests { ComputeBudget::new(), ThisComputeMeter::new_ref(std::i64::MAX as u64), &mut ExecuteDetailsTimings::default(), - Arc::new(Accounts::default_for_tests()), - &ancestors, + &[], Hash::default(), 0, ); @@ -1253,8 +1220,7 @@ mod tests { ComputeBudget::new(), ThisComputeMeter::new_ref(std::i64::MAX as u64), &mut ExecuteDetailsTimings::default(), - Arc::new(Accounts::default_for_tests()), - &ancestors, + &[], Hash::default(), 0, ); @@ -1272,7 +1238,6 @@ mod tests { )], Some(&accounts[0].0), ); - let ancestors = Ancestors::default(); let result = MessageProcessor::process_message( &instruction_processor, &message, @@ -1286,8 +1251,7 @@ mod tests { ComputeBudget::new(), ThisComputeMeter::new_ref(std::i64::MAX as u64), &mut ExecuteDetailsTimings::default(), - Arc::new(Accounts::default_for_tests()), - &ancestors, + &[], Hash::default(), 0, ); @@ -1578,8 +1542,7 @@ mod tests { ComputeBudget::new(), ThisComputeMeter::new_ref(std::i64::MAX as u64), &mut ExecuteDetailsTimings::default(), - Arc::new(Accounts::default_for_tests()), - &Ancestors::default(), + &[], Hash::default(), 0, ); diff --git a/sdk/program/src/sysvar/mod.rs b/sdk/program/src/sysvar/mod.rs index bf08a239c..597ec2891 100644 --- a/sdk/program/src/sysvar/mod.rs +++ b/sdk/program/src/sysvar/mod.rs @@ -1,6 +1,7 @@ //! named accounts for synthesized data accounts for bank state, etc. //! use crate::{account_info::AccountInfo, program_error::ProgramError, pubkey::Pubkey}; +use lazy_static::lazy_static; pub mod clock; pub mod epoch_schedule; @@ -13,18 +14,25 @@ pub mod slot_hashes; pub mod slot_history; pub mod stake_history; -#[allow(deprecated)] +lazy_static! { + pub static ref ALL_IDS: Vec = vec![ + clock::id(), + epoch_schedule::id(), + #[allow(deprecated)] + fees::id(), + #[allow(deprecated)] + recent_blockhashes::id(), + rent::id(), + rewards::id(), + slot_hashes::id(), + slot_history::id(), + stake_history::id(), + instructions::id(), + ]; +} + pub fn is_sysvar_id(id: &Pubkey) -> bool { - clock::check_id(id) - || epoch_schedule::check_id(id) - || fees::check_id(id) - || recent_blockhashes::check_id(id) - || rent::check_id(id) - || rewards::check_id(id) - || slot_hashes::check_id(id) - || slot_history::check_id(id) - || stake_history::check_id(id) - || instructions::check_id(id) + ALL_IDS.iter().any(|key| key == id) } #[macro_export] diff --git a/sdk/src/process_instruction.rs b/sdk/src/process_instruction.rs index c8fea8033..2f146d0a8 100644 --- a/sdk/src/process_instruction.rs +++ b/sdk/src/process_instruction.rs @@ -121,10 +121,7 @@ pub trait InvokeContext { deserialize_us: u64, ); /// Get sysvars - #[allow(clippy::type_complexity)] - fn get_sysvars(&self) -> &RefCell>>)>>; - /// Get sysvar data - fn get_sysvar_data(&self, id: &Pubkey) -> Option>>; + fn get_sysvars(&self) -> &[(Pubkey, Vec)]; /// Get this invocation's compute budget fn get_compute_budget(&self) -> &ComputeBudget; /// Set this invocation's blockhash @@ -175,15 +172,20 @@ pub fn get_sysvar( invoke_context: &dyn InvokeContext, id: &Pubkey, ) -> Result { - let sysvar_data = invoke_context.get_sysvar_data(id).ok_or_else(|| { - ic_msg!(invoke_context, "Unable to get sysvar {}", id); - InstructionError::UnsupportedSysvar - })?; - - bincode::deserialize(&sysvar_data).map_err(|err| { - ic_msg!(invoke_context, "Unable to get sysvar {}: {:?}", id, err); - InstructionError::UnsupportedSysvar - }) + invoke_context + .get_sysvars() + .iter() + .find_map(|(key, data)| { + if id == key { + bincode::deserialize(data).ok() + } else { + None + } + }) + .ok_or_else(|| { + ic_msg!(invoke_context, "Unable to get sysvar {}", id); + InstructionError::UnsupportedSysvar + }) } /// Compute meter @@ -356,8 +358,7 @@ pub struct MockInvokeContext<'a> { pub compute_meter: Rc>, pub programs: Vec<(Pubkey, ProcessInstructionWithContext)>, pub accounts: Vec<(Pubkey, Rc>)>, - #[allow(clippy::type_complexity)] - pub sysvars: RefCell>>)>>, + pub sysvars: &'a [(Pubkey, Vec)], pub disabled_features: HashSet, pub blockhash: Hash, pub lamports_per_signature: u64, @@ -376,7 +377,7 @@ impl<'a> MockInvokeContext<'a> { })), programs: vec![], accounts: vec![], - sysvars: RefCell::new(Vec::new()), + sysvars: &[], disabled_features: HashSet::default(), blockhash: Hash::default(), lamports_per_signature: 0, @@ -490,15 +491,8 @@ impl<'a> InvokeContext for MockInvokeContext<'a> { _deserialize_us: u64, ) { } - #[allow(clippy::type_complexity)] - fn get_sysvars(&self) -> &RefCell>>)>> { - &self.sysvars - } - fn get_sysvar_data(&self, id: &Pubkey) -> Option>> { + fn get_sysvars(&self) -> &[(Pubkey, Vec)] { self.sysvars - .borrow() - .iter() - .find_map(|(key, sysvar)| if id == key { sysvar.clone() } else { None }) } fn get_compute_budget(&self) -> &ComputeBudget { &self.compute_budget