Skip to content

Commit

Permalink
SVM: Dissolve RuntimeConfig (anza-xyz#1590)
Browse files Browse the repository at this point in the history
* SVM: add `compute_budget` to processing config

* bank: add `compute_budget` field

* SVM: add `transaction_account_lock_limit` to processing config

* bank: add `transaction_account_lock_limit` field

* bank: drop `runtime_config` getter

* SVM: require `compute_budget` for `prepare_program_cache_for_upcoming_feature_set`

* SVM: drop `runtime_config` from batch processor
  • Loading branch information
buffalojoec authored Jun 12, 2024
1 parent 8d3b2c9 commit ecf7252
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 50 deletions.
2 changes: 2 additions & 0 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,11 +606,13 @@ impl Consumer {
&mut execute_and_commit_timings.execute_timings,
TransactionProcessingConfig {
account_overrides: None,
compute_budget: bank.compute_budget(),
log_messages_bytes_limit: self.log_messages_bytes_limit,
limit_to_load_programs: true,
recording_config: ExecutionRecordingConfig::new_single_setting(
transaction_status_sender_enabled
),
transaction_account_lock_limit: Some(bank.get_transaction_account_lock_limit()),
}
));
execute_and_commit_timings.load_execute_us = load_execute_us;
Expand Down
43 changes: 31 additions & 12 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ use {
storable_accounts::StorableAccounts,
},
solana_bpf_loader_program::syscalls::create_program_runtime_environment_v1,
solana_compute_budget::compute_budget_processor::process_compute_budget_instructions,
solana_compute_budget::{
compute_budget::ComputeBudget,
compute_budget_processor::process_compute_budget_instructions,
},
solana_cost_model::cost_tracker::CostTracker,
solana_loader_v4_program::create_program_runtime_environment_v2,
solana_measure::{measure, measure::Measure, measure_us},
Expand Down Expand Up @@ -595,6 +598,8 @@ impl PartialEq for Bank {
transaction_processor: _,
check_program_modification_slot: _,
collector_fee_details: _,
compute_budget: _,
transaction_account_lock_limit: _,
// Ignore new fields explicitly if they do not impact PartialEq.
// Adding ".." will remove compile-time checks that if a new field
// is added to the struct, this PartialEq is accordingly updated.
Expand Down Expand Up @@ -874,6 +879,12 @@ pub struct Bank {

/// Collected fee details
collector_fee_details: RwLock<CollectorFeeDetails>,

/// The compute budget to use for transaction execution.
compute_budget: Option<ComputeBudget>,

/// The max number of accounts that a transaction may lock.
transaction_account_lock_limit: Option<usize>,
}

struct VoteWithStakeDelegations {
Expand Down Expand Up @@ -989,13 +1000,14 @@ impl Bank {
transaction_processor: TransactionBatchProcessor::default(),
check_program_modification_slot: false,
collector_fee_details: RwLock::new(CollectorFeeDetails::default()),
compute_budget: None,
transaction_account_lock_limit: None,
};

bank.transaction_processor = TransactionBatchProcessor::new(
bank.slot,
bank.epoch,
bank.epoch_schedule.clone(),
Arc::new(RuntimeConfig::default()),
HashSet::default(),
);

Expand Down Expand Up @@ -1032,8 +1044,9 @@ impl Bank {
let accounts = Accounts::new(Arc::new(accounts_db));
let mut bank = Self::default_with_accounts(accounts);
bank.ancestors = Ancestors::from(vec![bank.slot()]);
bank.compute_budget = runtime_config.compute_budget;
bank.transaction_account_lock_limit = runtime_config.transaction_account_lock_limit;
bank.transaction_debug_keys = debug_keys;
bank.transaction_processor.runtime_config = runtime_config;
bank.cluster_type = Some(genesis_config.cluster_type);

#[cfg(not(feature = "dev-context-only-utils"))]
Expand Down Expand Up @@ -1236,6 +1249,8 @@ impl Bank {
transaction_processor,
check_program_modification_slot: false,
collector_fee_details: RwLock::new(CollectorFeeDetails::default()),
compute_budget: parent.compute_budget,
transaction_account_lock_limit: parent.transaction_account_lock_limit,
};

let (_, ancestors_time_us) = measure_us!({
Expand Down Expand Up @@ -1270,7 +1285,8 @@ impl Bank {
.transaction_processor
.prepare_program_cache_for_upcoming_feature_set(
&new,
&new.compute_active_feature_set(true).0
&new.compute_active_feature_set(true).0,
&new.compute_budget.unwrap_or_default(),
));

// Update sysvars before processing transactions
Expand Down Expand Up @@ -1619,13 +1635,14 @@ impl Bank {
check_program_modification_slot: false,
// collector_fee_details is not serialized to snapshot
collector_fee_details: RwLock::new(CollectorFeeDetails::default()),
compute_budget: runtime_config.compute_budget,
transaction_account_lock_limit: runtime_config.transaction_account_lock_limit,
};

bank.transaction_processor = TransactionBatchProcessor::new(
bank.slot,
bank.epoch,
bank.epoch_schedule.clone(),
runtime_config,
HashSet::default(),
);

Expand Down Expand Up @@ -3260,9 +3277,7 @@ impl Bank {

/// Get the max number of accounts that a transaction may lock in this block
pub fn get_transaction_account_lock_limit(&self) -> usize {
if let Some(transaction_account_lock_limit) =
self.runtime_config().transaction_account_lock_limit
{
if let Some(transaction_account_lock_limit) = self.transaction_account_lock_limit {
transaction_account_lock_limit
} else if self
.feature_set
Expand Down Expand Up @@ -3387,13 +3402,15 @@ impl Bank {
&mut timings,
TransactionProcessingConfig {
account_overrides: Some(&account_overrides),
compute_budget: self.compute_budget(),
log_messages_bytes_limit: None,
limit_to_load_programs: true,
recording_config: ExecutionRecordingConfig {
enable_cpi_recording,
enable_log_recording: true,
enable_return_data_recording: true,
},
transaction_account_lock_limit: Some(self.get_transaction_account_lock_limit()),
},
);

Expand Down Expand Up @@ -4857,9 +4874,11 @@ impl Bank {
timings,
TransactionProcessingConfig {
account_overrides: None,
compute_budget: self.compute_budget(),
log_messages_bytes_limit,
limit_to_load_programs: false,
recording_config,
transaction_account_lock_limit: Some(self.get_transaction_account_lock_limit()),
},
);

Expand Down Expand Up @@ -5183,15 +5202,15 @@ impl Bank {
program_cache.environments.program_runtime_v1 = Arc::new(
create_program_runtime_environment_v1(
&self.feature_set,
&self.runtime_config().compute_budget.unwrap_or_default(),
&self.compute_budget().unwrap_or_default(),
false, /* deployment */
false, /* debugging_features */
)
.unwrap(),
);
program_cache.environments.program_runtime_v2 =
Arc::new(create_program_runtime_environment_v2(
&self.runtime_config().compute_budget.unwrap_or_default(),
&self.compute_budget().unwrap_or_default(),
false, /* debugging_features */
));
}
Expand Down Expand Up @@ -6798,8 +6817,8 @@ impl Bank {
&self.transaction_processor.fee_structure
}

pub fn runtime_config(&self) -> &RuntimeConfig {
&self.transaction_processor.runtime_config
pub fn compute_budget(&self) -> Option<ComputeBudget> {
self.compute_budget
}

pub fn add_builtin(&self, program_id: Pubkey, name: &str, builtin: ProgramCacheEntry) {
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/bank/builtins/core_bpf_migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ impl Bank {
// environment, as well as the two `ProgramCacheForTxBatch`
// instances configured above, then invoke the loader.
{
let compute_budget = self.runtime_config().compute_budget.unwrap_or_default();
let compute_budget = self.compute_budget().unwrap_or_default();
let mut sysvar_cache = SysvarCache::default();
sysvar_cache.fill_missing_entries(|pubkey, set_sysvar| {
if let Some(account) = self.get_account(pubkey) {
Expand Down
60 changes: 27 additions & 33 deletions svm/src/transaction_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use {
message_processor::MessageProcessor,
nonce_info::NonceFull,
program_loader::load_program_with_pubkey,
runtime_config::RuntimeConfig,
transaction_account_state_info::TransactionAccountStateInfo,
transaction_error_metrics::TransactionErrorMetrics,
transaction_processing_callback::TransactionProcessingCallback,
Expand Down Expand Up @@ -106,13 +105,17 @@ pub struct TransactionProcessingConfig<'a> {
/// Encapsulates overridden accounts, typically used for transaction
/// simulation.
pub account_overrides: Option<&'a AccountOverrides>,
/// The compute budget to use for transaction execution.
pub compute_budget: Option<ComputeBudget>,
/// The maximum number of bytes that log messages can consume.
pub log_messages_bytes_limit: Option<usize>,
/// Whether to limit the number of programs loaded for the transaction
/// batch.
pub limit_to_load_programs: bool,
/// Recording capabilities for transaction execution.
pub recording_config: ExecutionRecordingConfig,
/// The max number of accounts that a transaction may lock.
pub transaction_account_lock_limit: Option<usize>,
}

#[cfg_attr(feature = "frozen-abi", derive(AbiExample))]
Expand All @@ -129,9 +132,6 @@ pub struct TransactionBatchProcessor<FG: ForkGraph> {
/// Transaction fee structure
pub fee_structure: FeeStructure,

/// Optional config parameters that can override runtime behavior
pub runtime_config: Arc<RuntimeConfig>,

/// SysvarCache is a collection of system variables that are
/// accessible from on chain programs. It is passed to SVM from
/// client code (e.g. Bank) and forwarded to the MessageProcessor.
Expand All @@ -151,7 +151,6 @@ impl<FG: ForkGraph> Debug for TransactionBatchProcessor<FG> {
.field("epoch", &self.epoch)
.field("epoch_schedule", &self.epoch_schedule)
.field("fee_structure", &self.fee_structure)
.field("runtime_config", &self.runtime_config)
.field("sysvar_cache", &self.sysvar_cache)
.field("program_cache", &self.program_cache)
.finish()
Expand All @@ -165,7 +164,6 @@ impl<FG: ForkGraph> Default for TransactionBatchProcessor<FG> {
epoch: Epoch::default(),
epoch_schedule: EpochSchedule::default(),
fee_structure: FeeStructure::default(),
runtime_config: Arc::<RuntimeConfig>::default(),
sysvar_cache: RwLock::<SysvarCache>::default(),
program_cache: Arc::new(RwLock::new(ProgramCache::new(
Slot::default(),
Expand All @@ -181,15 +179,13 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
slot: Slot,
epoch: Epoch,
epoch_schedule: EpochSchedule,
runtime_config: Arc<RuntimeConfig>,
builtin_program_ids: HashSet<Pubkey>,
) -> Self {
Self {
slot,
epoch,
epoch_schedule,
fee_structure: FeeStructure::default(),
runtime_config,
sysvar_cache: RwLock::<SysvarCache>::default(),
program_cache: Arc::new(RwLock::new(ProgramCache::new(slot, epoch))),
builtin_program_ids: RwLock::new(builtin_program_ids),
Expand All @@ -202,7 +198,6 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
epoch,
epoch_schedule: self.epoch_schedule.clone(),
fee_structure: self.fee_structure.clone(),
runtime_config: self.runtime_config.clone(),
sysvar_cache: RwLock::<SysvarCache>::default(),
program_cache: self.program_cache.clone(),
builtin_program_ids: RwLock::new(self.builtin_program_ids.read().unwrap().clone()),
Expand Down Expand Up @@ -288,27 +283,26 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
.map(|(load_result, tx)| match load_result {
Err(e) => TransactionExecutionResult::NotExecuted(e.clone()),
Ok(loaded_transaction) => {
let compute_budget =
if let Some(compute_budget) = self.runtime_config.compute_budget {
compute_budget
} else {
let mut compute_budget_process_transaction_time =
Measure::start("compute_budget_process_transaction_time");
let maybe_compute_budget = ComputeBudget::try_from_instructions(
tx.message().program_instructions_iter(),
);
compute_budget_process_transaction_time.stop();
saturating_add_assign!(
execute_timings
.execute_accessories
.compute_budget_process_transaction_us,
compute_budget_process_transaction_time.as_us()
);
if let Err(err) = maybe_compute_budget {
return TransactionExecutionResult::NotExecuted(err);
}
maybe_compute_budget.unwrap()
};
let compute_budget = if let Some(compute_budget) = config.compute_budget {
compute_budget
} else {
let mut compute_budget_process_transaction_time =
Measure::start("compute_budget_process_transaction_time");
let maybe_compute_budget = ComputeBudget::try_from_instructions(
tx.message().program_instructions_iter(),
);
compute_budget_process_transaction_time.stop();
saturating_add_assign!(
execute_timings
.execute_accessories
.compute_budget_process_transaction_us,
compute_budget_process_transaction_time.as_us()
);
if let Err(err) = maybe_compute_budget {
return TransactionExecutionResult::NotExecuted(err);
}
maybe_compute_budget.unwrap()
};

let result = self.execute_loaded_transaction(
callbacks,
Expand Down Expand Up @@ -610,6 +604,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
&self,
callbacks: &CB,
upcoming_feature_set: &FeatureSet,
compute_budget: &ComputeBudget,
) {
// Recompile loaded programs one at a time before the next epoch hits
let (_epoch, slot_index) = self.epoch_schedule.get_epoch_and_slot_index(self.slot);
Expand Down Expand Up @@ -653,13 +648,13 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
let mut program_cache = self.program_cache.write().unwrap();
let program_runtime_environment_v1 = create_program_runtime_environment_v1(
upcoming_feature_set,
&self.runtime_config.compute_budget.unwrap_or_default(),
compute_budget,
false, /* deployment */
false, /* debugging_features */
)
.unwrap();
let program_runtime_environment_v2 = create_program_runtime_environment_v2(
&self.runtime_config.compute_budget.unwrap_or_default(),
compute_budget,
false, /* debugging_features */
);
let mut upcoming_environments = program_cache.environments.clone();
Expand Down Expand Up @@ -1853,7 +1848,6 @@ mod tests {
5,
5,
EpochSchedule::default(),
Arc::new(RuntimeConfig::default()),
HashSet::new(),
);
batch_processor.program_cache.write().unwrap().fork_graph =
Expand Down
4 changes: 2 additions & 2 deletions svm/tests/conformance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ use {
solana_svm::{
account_loader::CheckedTransactionDetails,
program_loader,
runtime_config::RuntimeConfig,
transaction_processing_callback::TransactionProcessingCallback,
transaction_processor::{
ExecutionRecordingConfig, TransactionBatchProcessor, TransactionProcessingConfig,
Expand Down Expand Up @@ -251,7 +250,6 @@ fn run_fixture(fixture: InstrFixture, filename: OsString, execute_as_instr: bool
42,
2,
EpochSchedule::default(),
Arc::new(RuntimeConfig::default()),
HashSet::new(),
);

Expand Down Expand Up @@ -291,9 +289,11 @@ fn run_fixture(fixture: InstrFixture, filename: OsString, execute_as_instr: bool
};
let processor_config = TransactionProcessingConfig {
account_overrides: None,
compute_budget: None,
log_messages_bytes_limit: None,
limit_to_load_programs: true,
recording_config,
transaction_account_lock_limit: None,
};

if execute_as_instr {
Expand Down
2 changes: 0 additions & 2 deletions svm/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use {
},
solana_svm::{
account_loader::{CheckedTransactionDetails, TransactionCheckResult},
runtime_config::RuntimeConfig,
transaction_processing_callback::TransactionProcessingCallback,
transaction_processor::{
ExecutionRecordingConfig, TransactionBatchProcessor, TransactionProcessingConfig,
Expand Down Expand Up @@ -446,7 +445,6 @@ fn svm_integration() {
EXECUTION_SLOT,
EXECUTION_EPOCH,
EpochSchedule::default(),
Arc::new(RuntimeConfig::default()),
HashSet::new(),
);

Expand Down

0 comments on commit ecf7252

Please sign in to comment.