Skip to content

Commit

Permalink
Refactor - Move Executor in program-runtime crate (solana-labs#28782)
Browse files Browse the repository at this point in the history
* Moves CreateMetrics into the program-runtime crate.

* Moves the Executor trait into executor.rs

* Removes the first_instruction_account parameter from Executor::execute().
  • Loading branch information
Lichtso authored Nov 10, 2022
1 parent 628ac0d commit 4142f42
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 70 deletions.
39 changes: 39 additions & 0 deletions program-runtime/src/executor.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use {
crate::{invoke_context::InvokeContext, timings::ExecuteDetailsTimings},
solana_sdk::{instruction::InstructionError, saturating_add_assign},
};

/// Program executor
pub trait Executor: std::fmt::Debug + Send + Sync {
/// Execute the program
fn execute(&self, invoke_context: &mut InvokeContext) -> Result<(), InstructionError>;
}

#[derive(Debug, Default)]
pub struct CreateMetrics {
pub program_id: String,
pub register_syscalls_us: u64,
pub load_elf_us: u64,
pub verify_code_us: u64,
pub jit_compile_us: u64,
}

impl CreateMetrics {
pub fn submit_datapoint(&self, timings: &mut ExecuteDetailsTimings) {
saturating_add_assign!(
timings.create_executor_register_syscalls_us,
self.register_syscalls_us
);
saturating_add_assign!(timings.create_executor_load_elf_us, self.load_elf_us);
saturating_add_assign!(timings.create_executor_verify_code_us, self.verify_code_us);
saturating_add_assign!(timings.create_executor_jit_compile_us, self.jit_compile_us);
datapoint_trace!(
"create_executor_trace",
("program_id", self.program_id, String),
("register_syscalls_us", self.register_syscalls_us, i64),
("load_elf_us", self.load_elf_us, i64),
("verify_code_us", self.verify_code_us, i64),
("jit_compile_us", self.jit_compile_us, i64),
);
}
}
18 changes: 2 additions & 16 deletions program-runtime/src/executor_cache.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
use {
crate::invoke_context::InvokeContext,
crate::executor::Executor,
log::*,
rand::Rng,
solana_sdk::{
instruction::InstructionError, pubkey::Pubkey, saturating_add_assign, slot_history::Slot,
stake_history::Epoch, transaction_context::IndexOfAccount,
},
solana_sdk::{pubkey::Pubkey, saturating_add_assign, slot_history::Slot, stake_history::Epoch},
std::{
collections::HashMap,
fmt::Debug,
Expand All @@ -17,16 +14,6 @@ use {
},
};

/// Program executor
pub trait Executor: Debug + Send + Sync {
/// Execute the program
fn execute(
&self,
first_instruction_account: IndexOfAccount,
invoke_context: &mut InvokeContext,
) -> Result<(), InstructionError>;
}

/// Relation between a TransactionExecutorCacheEntry and its matching BankExecutorCacheEntry
#[repr(u8)]
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -386,7 +373,6 @@ mod tests {
impl Executor for TestExecutor {
fn execute(
&self,
_first_instruction_account: IndexOfAccount,
_invoke_context: &mut InvokeContext,
) -> std::result::Result<(), InstructionError> {
Ok(())
Expand Down
1 change: 1 addition & 0 deletions program-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ extern crate solana_metrics;

pub mod accounts_data_meter;
pub mod compute_budget;
pub mod executor;
pub mod executor_cache;
pub mod invoke_context;
pub mod log_collector;
Expand Down
62 changes: 11 additions & 51 deletions programs/bpf_loader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ pub mod upgradeable;
pub mod upgradeable_with_jit;
pub mod with_jit;

#[macro_use]
extern crate solana_metrics;

use {
crate::{
allocator_bump::BpfAllocator,
Expand All @@ -22,13 +19,13 @@ use {
solana_measure::measure::Measure,
solana_program_runtime::{
compute_budget::ComputeBudget,
executor_cache::{Executor, TransactionExecutorCache},
executor::{CreateMetrics, Executor},
executor_cache::TransactionExecutorCache,
ic_logger_msg, ic_msg,
invoke_context::{ComputeMeter, InvokeContext},
log_collector::LogCollector,
stable_log,
sysvar_cache::get_sysvar_with_account_check,
timings::ExecuteDetailsTimings,
},
solana_rbpf::{
aligned_memory::AlignedMemory,
Expand Down Expand Up @@ -91,39 +88,6 @@ pub enum BpfError {
}
impl UserDefinedError for BpfError {}

mod executor_metrics {
use super::*;

#[derive(Debug, Default)]
pub struct CreateMetrics {
pub program_id: String,
pub register_syscalls_us: u64,
pub load_elf_us: u64,
pub verify_code_us: u64,
pub jit_compile_us: u64,
}

impl CreateMetrics {
pub fn submit_datapoint(&self, timings: &mut ExecuteDetailsTimings) {
saturating_add_assign!(
timings.create_executor_register_syscalls_us,
self.register_syscalls_us
);
saturating_add_assign!(timings.create_executor_load_elf_us, self.load_elf_us);
saturating_add_assign!(timings.create_executor_verify_code_us, self.verify_code_us);
saturating_add_assign!(timings.create_executor_jit_compile_us, self.jit_compile_us);
datapoint_trace!(
"create_executor_trace",
("program_id", self.program_id, String),
("register_syscalls_us", self.register_syscalls_us, i64),
("load_elf_us", self.load_elf_us, i64),
("verify_code_us", self.verify_code_us, i64),
("jit_compile_us", self.jit_compile_us, i64),
);
}
}
}

// The BPF loader is special in that it is the only place in the runtime and its built-in programs,
// where data comes not only from instruction account but also program accounts.
// Thus, these two helper methods have to distinguish the mixed sources via index_in_instruction.
Expand Down Expand Up @@ -162,7 +126,7 @@ fn create_executor_from_bytes(
feature_set: &FeatureSet,
compute_budget: &ComputeBudget,
log_collector: Option<Rc<RefCell<LogCollector>>>,
create_executor_metrics: &mut executor_metrics::CreateMetrics,
create_executor_metrics: &mut CreateMetrics,
programdata: &[u8],
use_jit: bool,
reject_deployment_of_broken_elfs: bool,
Expand Down Expand Up @@ -246,7 +210,7 @@ pub fn create_executor_from_account(
program: &BorrowedAccount,
programdata: &BorrowedAccount,
use_jit: bool,
) -> Result<(Arc<dyn Executor>, Option<executor_metrics::CreateMetrics>), InstructionError> {
) -> Result<(Arc<dyn Executor>, Option<CreateMetrics>), InstructionError> {
if !check_loader_id(program.get_owner()) {
ic_logger_msg!(
log_collector,
Expand Down Expand Up @@ -292,9 +256,9 @@ pub fn create_executor_from_account(
}
}

let mut create_executor_metrics = executor_metrics::CreateMetrics {
let mut create_executor_metrics = CreateMetrics {
program_id: program.get_key().to_string(),
..executor_metrics::CreateMetrics::default()
..CreateMetrics::default()
};
let executor = create_executor_from_bytes(
feature_set,
Expand Down Expand Up @@ -489,7 +453,7 @@ fn process_instruction_common(
create_executor_metrics.submit_datapoint(&mut invoke_context.timings);
}

executor.execute(program_account_index, invoke_context)
executor.execute(invoke_context)
} else {
drop(program);
debug_assert_eq!(first_instruction_account, 1);
Expand Down Expand Up @@ -702,7 +666,7 @@ fn process_loader_upgradeable_instruction(
let instruction_context = transaction_context.get_current_instruction_context()?;
let buffer =
instruction_context.try_borrow_instruction_account(transaction_context, 3)?;
let mut create_executor_metrics = executor_metrics::CreateMetrics::default();
let mut create_executor_metrics = CreateMetrics::default();
let executor = create_executor_from_bytes(
&invoke_context.feature_set,
invoke_context.get_compute_budget(),
Expand Down Expand Up @@ -881,7 +845,7 @@ fn process_loader_upgradeable_instruction(
// Load and verify the program bits
let buffer =
instruction_context.try_borrow_instruction_account(transaction_context, 2)?;
let mut create_executor_metrics = executor_metrics::CreateMetrics::default();
let mut create_executor_metrics = CreateMetrics::default();
let executor = create_executor_from_bytes(
&invoke_context.feature_set,
invoke_context.get_compute_budget(),
Expand Down Expand Up @@ -1378,7 +1342,7 @@ fn process_loader_instruction(
ic_msg!(invoke_context, "key[0] did not sign the transaction");
return Err(InstructionError::MissingRequiredSignature);
}
let mut create_executor_metrics = executor_metrics::CreateMetrics::default();
let mut create_executor_metrics = CreateMetrics::default();
let executor = create_executor_from_bytes(
&invoke_context.feature_set,
invoke_context.get_compute_budget(),
Expand Down Expand Up @@ -1436,11 +1400,7 @@ impl Debug for BpfExecutor {
}

impl Executor for BpfExecutor {
fn execute(
&self,
_first_instruction_account: IndexOfAccount,
invoke_context: &mut InvokeContext,
) -> Result<(), InstructionError> {
fn execute(&self, invoke_context: &mut InvokeContext) -> Result<(), InstructionError> {
let log_collector = invoke_context.get_log_collector();
let compute_meter = invoke_context.get_compute_meter();
let stack_height = invoke_context.get_stack_height();
Expand Down
5 changes: 2 additions & 3 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ use {
solana_program_runtime::{
accounts_data_meter::MAX_ACCOUNTS_DATA_LEN,
compute_budget::{self, ComputeBudget},
executor::Executor,
executor_cache::{
BankExecutorCache, Executor, TransactionExecutorCache, TxBankExecutorCacheDiff,
BankExecutorCache, TransactionExecutorCache, TxBankExecutorCacheDiff,
MAX_CACHED_EXECUTORS,
},
invoke_context::{BuiltinProgram, ProcessInstructionWithContext},
Expand Down Expand Up @@ -7950,7 +7951,6 @@ pub(crate) mod tests {
rand::Rng,
solana_program_runtime::{
compute_budget::MAX_COMPUTE_UNIT_LIMIT,
executor_cache::Executor,
invoke_context::{mock_process_instruction, InvokeContext},
prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType},
},
Expand Down Expand Up @@ -15129,7 +15129,6 @@ pub(crate) mod tests {
impl Executor for TestExecutor {
fn execute(
&self,
_first_instruction_account: IndexOfAccount,
_invoke_context: &mut InvokeContext,
) -> std::result::Result<(), InstructionError> {
Ok(())
Expand Down

0 comments on commit 4142f42

Please sign in to comment.