* Perf: Only check executors cache for executable bpf program ids (#22624)
* Only check executors cache for executable bpf program ids
* switch to native loader check
* clean up tests
* fix tests
* clippy
(cherry picked from commit 7d34a7acac)
# Conflicts:
#	runtime/src/bank.rs
* resolve conflicts
Co-authored-by: Justin Starry <justin@solana.com>
			
			
This commit is contained in:
		@@ -3584,33 +3584,32 @@ impl Bank {
 | 
				
			|||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    /// Get any cached executors needed by the transaction
 | 
					    /// Get any cached executors needed by the transaction
 | 
				
			||||||
    fn get_executors(
 | 
					    fn get_executors(&self, accounts: &[(Pubkey, AccountSharedData)]) -> Rc<RefCell<Executors>> {
 | 
				
			||||||
        &self,
 | 
					        let executable_keys: Vec<_> = accounts
 | 
				
			||||||
        message: &SanitizedMessage,
 | 
					            .iter()
 | 
				
			||||||
        accounts: &[(Pubkey, AccountSharedData)],
 | 
					            .filter_map(|(key, account)| {
 | 
				
			||||||
        program_indices: &[Vec<usize>],
 | 
					                if account.executable() && !native_loader::check_id(account.owner()) {
 | 
				
			||||||
    ) -> Rc<RefCell<Executors>> {
 | 
					                    Some(key)
 | 
				
			||||||
        let mut num_executors = message.account_keys_len();
 | 
					                } else {
 | 
				
			||||||
        for program_indices_of_instruction in program_indices.iter() {
 | 
					                    None
 | 
				
			||||||
            num_executors += program_indices_of_instruction.len();
 | 
					 | 
				
			||||||
                }
 | 
					                }
 | 
				
			||||||
        let mut executors = HashMap::with_capacity(num_executors);
 | 
					            })
 | 
				
			||||||
        let cache = self.cached_executors.read().unwrap();
 | 
					            .collect();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        for key in message.account_keys_iter() {
 | 
					        if executable_keys.is_empty() {
 | 
				
			||||||
            if let Some(executor) = cache.get(key) {
 | 
					            return Rc::new(RefCell::new(Executors::default()));
 | 
				
			||||||
                executors.insert(*key, TransactionExecutor::new_cached(executor));
 | 
					 | 
				
			||||||
            }
 | 
					 | 
				
			||||||
        }
 | 
					 | 
				
			||||||
        for program_indices_of_instruction in program_indices.iter() {
 | 
					 | 
				
			||||||
            for account_index in program_indices_of_instruction.iter() {
 | 
					 | 
				
			||||||
                let key = accounts[*account_index].0;
 | 
					 | 
				
			||||||
                if let Some(executor) = cache.get(&key) {
 | 
					 | 
				
			||||||
                    executors.insert(key, TransactionExecutor::new_cached(executor));
 | 
					 | 
				
			||||||
                }
 | 
					 | 
				
			||||||
            }
 | 
					 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        let cache = self.cached_executors.read().unwrap();
 | 
				
			||||||
 | 
					        let executors = executable_keys
 | 
				
			||||||
 | 
					            .into_iter()
 | 
				
			||||||
 | 
					            .filter_map(|key| {
 | 
				
			||||||
 | 
					                cache
 | 
				
			||||||
 | 
					                    .get(key)
 | 
				
			||||||
 | 
					                    .map(|executor| (*key, TransactionExecutor::new_cached(executor)))
 | 
				
			||||||
 | 
					            })
 | 
				
			||||||
 | 
					            .collect();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        Rc::new(RefCell::new(executors))
 | 
					        Rc::new(RefCell::new(executors))
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -3682,11 +3681,7 @@ impl Bank {
 | 
				
			|||||||
        error_counters: &mut ErrorCounters,
 | 
					        error_counters: &mut ErrorCounters,
 | 
				
			||||||
    ) -> TransactionExecutionResult {
 | 
					    ) -> TransactionExecutionResult {
 | 
				
			||||||
        let mut get_executors_time = Measure::start("get_executors_time");
 | 
					        let mut get_executors_time = Measure::start("get_executors_time");
 | 
				
			||||||
        let executors = self.get_executors(
 | 
					        let executors = self.get_executors(&loaded_transaction.accounts);
 | 
				
			||||||
            tx.message(),
 | 
					 | 
				
			||||||
            &loaded_transaction.accounts,
 | 
					 | 
				
			||||||
            &loaded_transaction.program_indices,
 | 
					 | 
				
			||||||
        );
 | 
					 | 
				
			||||||
        get_executors_time.stop();
 | 
					        get_executors_time.stop();
 | 
				
			||||||
        saturating_add_assign!(
 | 
					        saturating_add_assign!(
 | 
				
			||||||
            timings.execute_accessories.get_executors_us,
 | 
					            timings.execute_accessories.get_executors_us,
 | 
				
			||||||
@@ -6440,6 +6435,7 @@ pub(crate) mod tests {
 | 
				
			|||||||
        solana_program_runtime::invoke_context::InvokeContext,
 | 
					        solana_program_runtime::invoke_context::InvokeContext,
 | 
				
			||||||
        solana_sdk::{
 | 
					        solana_sdk::{
 | 
				
			||||||
            account::Account,
 | 
					            account::Account,
 | 
				
			||||||
 | 
					            bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable,
 | 
				
			||||||
            clock::{DEFAULT_SLOTS_PER_EPOCH, DEFAULT_TICKS_PER_SLOT},
 | 
					            clock::{DEFAULT_SLOTS_PER_EPOCH, DEFAULT_TICKS_PER_SLOT},
 | 
				
			||||||
            compute_budget::ComputeBudgetInstruction,
 | 
					            compute_budget::ComputeBudgetInstruction,
 | 
				
			||||||
            epoch_schedule::MINIMUM_SLOTS_PER_EPOCH,
 | 
					            epoch_schedule::MINIMUM_SLOTS_PER_EPOCH,
 | 
				
			||||||
@@ -13037,26 +13033,23 @@ pub(crate) mod tests {
 | 
				
			|||||||
        let key2 = solana_sdk::pubkey::new_rand();
 | 
					        let key2 = solana_sdk::pubkey::new_rand();
 | 
				
			||||||
        let key3 = solana_sdk::pubkey::new_rand();
 | 
					        let key3 = solana_sdk::pubkey::new_rand();
 | 
				
			||||||
        let key4 = solana_sdk::pubkey::new_rand();
 | 
					        let key4 = solana_sdk::pubkey::new_rand();
 | 
				
			||||||
 | 
					        let key5 = solana_sdk::pubkey::new_rand();
 | 
				
			||||||
        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
 | 
					        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        let message = Message {
 | 
					        fn new_executable_account(owner: Pubkey) -> AccountSharedData {
 | 
				
			||||||
            header: MessageHeader {
 | 
					            AccountSharedData::from(Account {
 | 
				
			||||||
                num_required_signatures: 1,
 | 
					                owner,
 | 
				
			||||||
                num_readonly_signed_accounts: 0,
 | 
					                executable: true,
 | 
				
			||||||
                num_readonly_unsigned_accounts: 1,
 | 
					                ..Account::default()
 | 
				
			||||||
            },
 | 
					            })
 | 
				
			||||||
            account_keys: vec![key1, key2],
 | 
					 | 
				
			||||||
            recent_blockhash: Hash::default(),
 | 
					 | 
				
			||||||
            instructions: vec![],
 | 
					 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
        .try_into()
 | 
					 | 
				
			||||||
        .unwrap();
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
        let program_indices = &[vec![0, 1], vec![2]];
 | 
					 | 
				
			||||||
        let accounts = &[
 | 
					        let accounts = &[
 | 
				
			||||||
            (key3, AccountSharedData::default()),
 | 
					            (key1, new_executable_account(bpf_loader_upgradeable::id())),
 | 
				
			||||||
            (key4, AccountSharedData::default()),
 | 
					            (key2, new_executable_account(bpf_loader::id())),
 | 
				
			||||||
            (key1, AccountSharedData::default()),
 | 
					            (key3, new_executable_account(bpf_loader_deprecated::id())),
 | 
				
			||||||
 | 
					            (key4, new_executable_account(native_loader::id())),
 | 
				
			||||||
 | 
					            (key5, AccountSharedData::default()),
 | 
				
			||||||
        ];
 | 
					        ];
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        // don't do any work if not dirty
 | 
					        // don't do any work if not dirty
 | 
				
			||||||
@@ -13072,7 +13065,7 @@ pub(crate) mod tests {
 | 
				
			|||||||
            .unwrap()
 | 
					            .unwrap()
 | 
				
			||||||
            .clear_miss_for_test();
 | 
					            .clear_miss_for_test();
 | 
				
			||||||
        bank.update_executors(true, executors);
 | 
					        bank.update_executors(true, executors);
 | 
				
			||||||
        let executors = bank.get_executors(&message, accounts, program_indices);
 | 
					        let executors = bank.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 0);
 | 
					        assert_eq!(executors.borrow().len(), 0);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        // do work
 | 
					        // do work
 | 
				
			||||||
@@ -13083,33 +13076,27 @@ pub(crate) mod tests {
 | 
				
			|||||||
        executors.insert(key4, TransactionExecutor::new_miss(executor.clone()));
 | 
					        executors.insert(key4, TransactionExecutor::new_miss(executor.clone()));
 | 
				
			||||||
        let executors = Rc::new(RefCell::new(executors));
 | 
					        let executors = Rc::new(RefCell::new(executors));
 | 
				
			||||||
        bank.update_executors(true, executors);
 | 
					        bank.update_executors(true, executors);
 | 
				
			||||||
        let executors = bank.get_executors(&message, accounts, program_indices);
 | 
					        let executors = bank.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 4);
 | 
					        assert_eq!(executors.borrow().len(), 3);
 | 
				
			||||||
        assert!(executors.borrow().contains_key(&key1));
 | 
					        assert!(executors.borrow().contains_key(&key1));
 | 
				
			||||||
        assert!(executors.borrow().contains_key(&key2));
 | 
					        assert!(executors.borrow().contains_key(&key2));
 | 
				
			||||||
        assert!(executors.borrow().contains_key(&key3));
 | 
					        assert!(executors.borrow().contains_key(&key3));
 | 
				
			||||||
        assert!(executors.borrow().contains_key(&key4));
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
        // Check inheritance
 | 
					        // Check inheritance
 | 
				
			||||||
        let bank = Bank::new_from_parent(&Arc::new(bank), &solana_sdk::pubkey::new_rand(), 1);
 | 
					        let bank = Bank::new_from_parent(&Arc::new(bank), &solana_sdk::pubkey::new_rand(), 1);
 | 
				
			||||||
        let executors = bank.get_executors(&message, accounts, program_indices);
 | 
					        let executors = bank.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 4);
 | 
					        assert_eq!(executors.borrow().len(), 3);
 | 
				
			||||||
        assert!(executors.borrow().contains_key(&key1));
 | 
					        assert!(executors.borrow().contains_key(&key1));
 | 
				
			||||||
        assert!(executors.borrow().contains_key(&key2));
 | 
					        assert!(executors.borrow().contains_key(&key2));
 | 
				
			||||||
        assert!(executors.borrow().contains_key(&key3));
 | 
					        assert!(executors.borrow().contains_key(&key3));
 | 
				
			||||||
        assert!(executors.borrow().contains_key(&key4));
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
        // Remove all
 | 
					        // Remove all
 | 
				
			||||||
        bank.remove_executor(&key1);
 | 
					        bank.remove_executor(&key1);
 | 
				
			||||||
        bank.remove_executor(&key2);
 | 
					        bank.remove_executor(&key2);
 | 
				
			||||||
        bank.remove_executor(&key3);
 | 
					        bank.remove_executor(&key3);
 | 
				
			||||||
        bank.remove_executor(&key4);
 | 
					        bank.remove_executor(&key4);
 | 
				
			||||||
        let executors = bank.get_executors(&message, accounts, program_indices);
 | 
					        let executors = bank.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 0);
 | 
					        assert_eq!(executors.borrow().len(), 0);
 | 
				
			||||||
        assert!(!executors.borrow().contains_key(&key1));
 | 
					 | 
				
			||||||
        assert!(!executors.borrow().contains_key(&key2));
 | 
					 | 
				
			||||||
        assert!(!executors.borrow().contains_key(&key3));
 | 
					 | 
				
			||||||
        assert!(!executors.borrow().contains_key(&key4));
 | 
					 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    #[test]
 | 
					    #[test]
 | 
				
			||||||
@@ -13122,13 +13109,15 @@ pub(crate) mod tests {
 | 
				
			|||||||
        let key1 = solana_sdk::pubkey::new_rand();
 | 
					        let key1 = solana_sdk::pubkey::new_rand();
 | 
				
			||||||
        let key2 = solana_sdk::pubkey::new_rand();
 | 
					        let key2 = solana_sdk::pubkey::new_rand();
 | 
				
			||||||
        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
 | 
					        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
 | 
				
			||||||
        let message =
 | 
					        let executable_account = AccountSharedData::from(Account {
 | 
				
			||||||
            SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap();
 | 
					            owner: bpf_loader_upgradeable::id(),
 | 
				
			||||||
 | 
					            executable: true,
 | 
				
			||||||
 | 
					            ..Account::default()
 | 
				
			||||||
 | 
					        });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        let program_indices = &[vec![0, 1]];
 | 
					 | 
				
			||||||
        let accounts = &[
 | 
					        let accounts = &[
 | 
				
			||||||
            (key1, AccountSharedData::default()),
 | 
					            (key1, executable_account.clone()),
 | 
				
			||||||
            (key2, AccountSharedData::default()),
 | 
					            (key2, executable_account),
 | 
				
			||||||
        ];
 | 
					        ];
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        // add one to root bank
 | 
					        // add one to root bank
 | 
				
			||||||
@@ -13136,15 +13125,15 @@ pub(crate) mod tests {
 | 
				
			|||||||
        executors.insert(key1, TransactionExecutor::new_miss(executor.clone()));
 | 
					        executors.insert(key1, TransactionExecutor::new_miss(executor.clone()));
 | 
				
			||||||
        let executors = Rc::new(RefCell::new(executors));
 | 
					        let executors = Rc::new(RefCell::new(executors));
 | 
				
			||||||
        root.update_executors(true, executors);
 | 
					        root.update_executors(true, executors);
 | 
				
			||||||
        let executors = root.get_executors(&message, accounts, program_indices);
 | 
					        let executors = root.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 1);
 | 
					        assert_eq!(executors.borrow().len(), 1);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        let fork1 = Bank::new_from_parent(&root, &Pubkey::default(), 1);
 | 
					        let fork1 = Bank::new_from_parent(&root, &Pubkey::default(), 1);
 | 
				
			||||||
        let fork2 = Bank::new_from_parent(&root, &Pubkey::default(), 1);
 | 
					        let fork2 = Bank::new_from_parent(&root, &Pubkey::default(), 2);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        let executors = fork1.get_executors(&message, accounts, program_indices);
 | 
					        let executors = fork1.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 1);
 | 
					        assert_eq!(executors.borrow().len(), 1);
 | 
				
			||||||
        let executors = fork2.get_executors(&message, accounts, program_indices);
 | 
					        let executors = fork2.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 1);
 | 
					        assert_eq!(executors.borrow().len(), 1);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        let mut executors = Executors::default();
 | 
					        let mut executors = Executors::default();
 | 
				
			||||||
@@ -13152,16 +13141,16 @@ pub(crate) mod tests {
 | 
				
			|||||||
        let executors = Rc::new(RefCell::new(executors));
 | 
					        let executors = Rc::new(RefCell::new(executors));
 | 
				
			||||||
        fork1.update_executors(true, executors);
 | 
					        fork1.update_executors(true, executors);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        let executors = fork1.get_executors(&message, accounts, program_indices);
 | 
					        let executors = fork1.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 2);
 | 
					        assert_eq!(executors.borrow().len(), 2);
 | 
				
			||||||
        let executors = fork2.get_executors(&message, accounts, program_indices);
 | 
					        let executors = fork2.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 1);
 | 
					        assert_eq!(executors.borrow().len(), 1);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        fork1.remove_executor(&key1);
 | 
					        fork1.remove_executor(&key1);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        let executors = fork1.get_executors(&message, accounts, program_indices);
 | 
					        let executors = fork1.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 1);
 | 
					        assert_eq!(executors.borrow().len(), 1);
 | 
				
			||||||
        let executors = fork2.get_executors(&message, accounts, program_indices);
 | 
					        let executors = fork2.get_executors(accounts);
 | 
				
			||||||
        assert_eq!(executors.borrow().len(), 1);
 | 
					        assert_eq!(executors.borrow().len(), 1);
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user