cache executors on failed transactions (backport #22308) (#22327)

* cache executors on failed transactions (#22308)

(cherry picked from commit 12e160269e)

# Conflicts:
#	program-runtime/src/invoke_context.rs
#	programs/bpf_loader/src/lib.rs
#	runtime/src/bank.rs

* resolve conflicts

Co-authored-by: Jack May <jack@solana.com>
This commit is contained in:
mergify[bot]
2022-01-06 15:38:39 -08:00
committed by GitHub
parent c90fa6643e
commit cad0e7f04f
5 changed files with 1764 additions and 40 deletions

File diff suppressed because it is too large Load Diff

View File

@ -441,7 +441,7 @@ fn process_loader_upgradeable_instruction(
// Load and verify the program bits
let executor = create_executor(3, buffer_data_offset, invoke_context, use_jit, true)?;
invoke_context.add_executor(&new_program_id, executor);
invoke_context.update_executor(&new_program_id, executor);
let keyed_accounts = invoke_context.get_keyed_accounts()?;
let payer = keyed_account_at_index(keyed_accounts, 0)?;
@ -573,7 +573,7 @@ fn process_loader_upgradeable_instruction(
// Load and verify the program bits
let executor = create_executor(2, buffer_data_offset, invoke_context, use_jit, true)?;
invoke_context.add_executor(&new_program_id, executor);
invoke_context.update_executor(&new_program_id, executor);
let keyed_accounts = invoke_context.get_keyed_accounts()?;
let programdata = keyed_account_at_index(keyed_accounts, 0)?;
@ -844,7 +844,7 @@ fn process_loader_instruction(
let executor = create_executor(0, 0, invoke_context, use_jit, true)?;
let keyed_accounts = invoke_context.get_keyed_accounts()?;
let program = keyed_account_at_index(keyed_accounts, 0)?;
invoke_context.add_executor(program.unsigned_key(), executor);
invoke_context.update_executor(program.unsigned_key(), executor);
program.try_account_ref_mut()?.set_executable(true);
ic_msg!(
invoke_context,

View File

@ -3336,13 +3336,13 @@ impl Bank {
for key in message.account_keys.iter() {
if let Some(executor) = cache.get(key) {
executors.insert(*key, TransactionExecutor::cached(executor));
executors.insert(*key, TransactionExecutor::new_cached(executor));
}
}
for instruction_loaders in loaders.iter() {
for (key, _) in instruction_loaders.iter() {
if let Some(executor) = cache.get(key) {
executors.insert(*key, TransactionExecutor::cached(executor));
executors.insert(*key, TransactionExecutor::new_cached(executor));
}
}
}
@ -3351,13 +3351,13 @@ impl Bank {
}
/// Add executors back to the bank's cache if modified
fn update_executors(&self, executors: Rc<RefCell<Executors>>) {
fn update_executors(&self, allow_updates: bool, executors: Rc<RefCell<Executors>>) {
let executors = executors.borrow();
let dirty_executors: Vec<_> = executors
.iter()
.filter_map(|(key, TransactionExecutor { executor, is_dirty })| {
if *is_dirty {
Some((key, executor.clone()))
.filter_map(|(key, executor)| {
if executor.is_dirty(allow_updates) {
Some((key, executor.get()))
} else {
None
}
@ -3531,9 +3531,7 @@ impl Bank {
process_result = Err(e);
}
if process_result.is_ok() {
self.update_executors(executors);
}
self.update_executors(process_result.is_ok(), executors);
} else {
transaction_log_messages.push(None);
inner_instructions.push(None);
@ -11969,24 +11967,28 @@ pub(crate) mod tests {
// don't do any work if not dirty
let mut executors = Executors::default();
executors.insert(key1, TransactionExecutor::cached(executor.clone()));
executors.insert(key2, TransactionExecutor::cached(executor.clone()));
executors.insert(key3, TransactionExecutor::cached(executor.clone()));
executors.insert(key4, TransactionExecutor::cached(executor.clone()));
executors.insert(key1, TransactionExecutor::new_cached(executor.clone()));
executors.insert(key2, TransactionExecutor::new_cached(executor.clone()));
executors.insert(key3, TransactionExecutor::new_cached(executor.clone()));
executors.insert(key4, TransactionExecutor::new_cached(executor.clone()));
let executors = Rc::new(RefCell::new(executors));
executors.borrow_mut().get_mut(&key1).unwrap().is_dirty = false;
bank.update_executors(executors);
executors
.borrow_mut()
.get_mut(&key1)
.unwrap()
.clear_miss_for_test();
bank.update_executors(true, executors);
let executors = bank.get_executors(&message, loaders);
assert_eq!(executors.borrow().len(), 0);
// do work
let mut executors = Executors::default();
executors.insert(key1, TransactionExecutor::dirty(executor.clone()));
executors.insert(key2, TransactionExecutor::dirty(executor.clone()));
executors.insert(key3, TransactionExecutor::dirty(executor.clone()));
executors.insert(key4, TransactionExecutor::dirty(executor.clone()));
executors.insert(key1, TransactionExecutor::new_miss(executor.clone()));
executors.insert(key2, TransactionExecutor::new_miss(executor.clone()));
executors.insert(key3, TransactionExecutor::new_miss(executor.clone()));
executors.insert(key4, TransactionExecutor::new_miss(executor.clone()));
let executors = Rc::new(RefCell::new(executors));
bank.update_executors(executors);
bank.update_executors(true, executors);
let executors = bank.get_executors(&message, loaders);
assert_eq!(executors.borrow().len(), 4);
assert!(executors.borrow().contains_key(&key1));
@ -12034,9 +12036,9 @@ pub(crate) mod tests {
// add one to root bank
let mut executors = Executors::default();
executors.insert(key1, TransactionExecutor::dirty(executor.clone()));
executors.insert(key1, TransactionExecutor::new_miss(executor.clone()));
let executors = Rc::new(RefCell::new(executors));
root.update_executors(executors);
root.update_executors(true, executors);
let executors = root.get_executors(&Message::default(), loaders);
assert_eq!(executors.borrow().len(), 1);
@ -12049,9 +12051,9 @@ pub(crate) mod tests {
assert_eq!(executors.borrow().len(), 1);
let mut executors = Executors::default();
executors.insert(key2, TransactionExecutor::dirty(executor.clone()));
executors.insert(key2, TransactionExecutor::new_miss(executor.clone()));
let executors = Rc::new(RefCell::new(executors));
fork1.update_executors(executors);
fork1.update_executors(true, executors);
let executors = fork1.get_executors(&Message::default(), loaders);
assert_eq!(executors.borrow().len(), 2);

View File

@ -41,31 +41,56 @@ use {
pub type Executors = HashMap<Pubkey, TransactionExecutor>;
/// Tracks whether a given executor is "dirty" and needs to
/// updated in the executors cache
/// Tracks whether a given executor is "dirty" and needs to updated in the
/// executors cache
pub struct TransactionExecutor {
pub executor: Arc<dyn Executor>,
pub is_dirty: bool,
executor: Arc<dyn Executor>,
is_miss: bool,
is_updated: bool,
}
impl TransactionExecutor {
/// Wraps an executor and tracks that it doesn't need
/// to be updated in the executors cache.
pub fn cached(executor: Arc<dyn Executor>) -> Self {
/// Wraps an executor and tracks that it doesn't need to be updated in the
/// executors cache.
pub fn new_cached(executor: Arc<dyn Executor>) -> Self {
Self {
executor,
is_dirty: false,
is_miss: false,
is_updated: false,
}
}
/// Wraps an executor and tracks that it needs to be
/// updated in the executors cache.
pub fn dirty(executor: Arc<dyn Executor>) -> Self {
/// Wraps an executor and tracks that it needs to be updated in the
/// executors cache.
pub fn new_miss(executor: Arc<dyn Executor>) -> Self {
Self {
executor,
is_dirty: true,
is_miss: true,
is_updated: false,
}
}
/// Wraps an executor and tracks that it needs to be updated in the
/// executors cache only if the transaction succeeded.
pub fn new_updated(executor: Arc<dyn Executor>) -> Self {
Self {
executor,
is_miss: false,
is_updated: true,
}
}
pub fn is_dirty(&self, include_updates: bool) -> bool {
self.is_miss || (include_updates && self.is_updated)
}
pub fn get(&self) -> Arc<dyn Executor> {
self.executor.clone()
}
pub fn clear_miss_for_test(&mut self) {
self.is_miss = false;
}
}
#[derive(Default, Debug, PartialEq)]
@ -540,7 +565,12 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
fn add_executor(&self, pubkey: &Pubkey, executor: Arc<dyn Executor>) {
self.executors
.borrow_mut()
.insert(*pubkey, TransactionExecutor::dirty(executor));
.insert(*pubkey, TransactionExecutor::new_miss(executor));
}
fn update_executor(&self, pubkey: &Pubkey, executor: Arc<dyn Executor>) {
self.executors
.borrow_mut()
.insert(*pubkey, TransactionExecutor::new_updated(executor));
}
fn get_executor(&self, pubkey: &Pubkey) -> Option<Arc<dyn Executor>> {
self.executors

View File

@ -85,6 +85,8 @@ pub trait InvokeContext {
/// Loaders may need to do work in order to execute a program. Cache
/// the work that can be re-used across executions
fn add_executor(&self, pubkey: &Pubkey, executor: Arc<dyn Executor>);
/// Cache an executor that has changed
fn update_executor(&self, pubkey: &Pubkey, executor: Arc<dyn Executor>);
/// Get the completed loader work that can be re-used across executions
fn get_executor(&self, pubkey: &Pubkey) -> Option<Arc<dyn Executor>>;
/// Record invoked instruction
@ -493,6 +495,7 @@ impl<'a> InvokeContext for MockInvokeContext<'a> {
Rc::new(RefCell::new(self.compute_meter.clone()))
}
fn add_executor(&self, _pubkey: &Pubkey, _executor: Arc<dyn Executor>) {}
fn update_executor(&self, _pubkey: &Pubkey, _executor: Arc<dyn Executor>) {}
fn get_executor(&self, _pubkey: &Pubkey) -> Option<Arc<dyn Executor>> {
None
}