Skip to content

Commit

Permalink
Execution layer reports timings of each executed command (#20923)
Browse files Browse the repository at this point in the history
The timings are unused for now - they are intended for the congestion
control execution time estimator.
  • Loading branch information
mystenmark authored Jan 24, 2025
1 parent 791ce91 commit 6210d3b
Show file tree
Hide file tree
Showing 18 changed files with 286 additions and 176 deletions.
32 changes: 17 additions & 15 deletions crates/simulacrum/src/epoch_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,20 +135,22 @@ impl EpochState {

let transaction_data = transaction.data().transaction_data();
let (kind, signer, gas) = transaction_data.execution_parts();
Ok(self.executor.execute_transaction_to_effects(
store.backing_store(),
&self.protocol_config,
self.limits_metrics.clone(),
false, // enable_expensive_checks
&HashSet::new(), // certificate_deny_set
&self.epoch_start_state.epoch(),
self.epoch_start_state.epoch_start_timestamp_ms(),
checked_input_objects,
gas,
gas_status,
kind,
signer,
tx_digest,
))
let (inner_temp_store, gas_status, effects, _timings, result) =
self.executor.execute_transaction_to_effects(
store.backing_store(),
&self.protocol_config,
self.limits_metrics.clone(),
false, // enable_expensive_checks
&HashSet::new(), // certificate_deny_set
&self.epoch_start_state.epoch(),
self.epoch_start_state.epoch_start_timestamp_ms(),
checked_input_objects,
gas,
gas_status,
kind,
signer,
tx_digest,
);
Ok((inner_temp_store, gas_status, effects, result))
}
}
6 changes: 3 additions & 3 deletions crates/sui-core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1655,7 +1655,7 @@ impl AuthorityState {
let (kind, signer, gas) = transaction_data.execution_parts();

#[allow(unused_mut)]
let (inner_temp_store, _, mut effects, execution_error_opt) =
let (inner_temp_store, _, mut effects, _timings, execution_error_opt) =
epoch_store.executor().execute_transaction_to_effects(
self.get_backing_store().as_ref(),
protocol_config,
Expand Down Expand Up @@ -1844,7 +1844,7 @@ impl AuthorityState {
.expect("Creating an executor should not fail here");

let expensive_checks = false;
let (inner_temp_store, _, effects, _execution_error) = executor
let (inner_temp_store, _, effects, _timings, _execution_error) = executor
.execute_transaction_to_effects(
self.get_backing_store().as_ref(),
protocol_config,
Expand Down Expand Up @@ -2034,7 +2034,7 @@ impl AuthorityState {
.expect("Creating an executor should not fail here");

let expensive_checks = false;
let (inner_temp_store, _, effects, _execution_error) = executor
let (inner_temp_store, _, effects, _timings, _execution_error) = executor
.execute_transaction_to_effects(
self.get_backing_store().as_ref(),
protocol_config,
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-genesis-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ fn create_genesis_transaction(
let transaction_data = &genesis_transaction.data().intent_message().value;
let (kind, signer, _) = transaction_data.execution_parts();
let input_objects = CheckedInputObjects::new_for_genesis(vec![]);
let (inner_temp_store, _, effects, _execution_error) = executor
let (inner_temp_store, _, effects, _timings, _execution_error) = executor
.execute_transaction_to_effects(
&InMemoryStorage::new(Vec::new()),
protocol_config,
Expand Down
33 changes: 17 additions & 16 deletions crates/sui-replay/src/replay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,21 +764,22 @@ impl LocalExec {
)
.expect("Failed to create gas status")
};
let (inner_store, gas_status, effects, result) = executor.execute_transaction_to_effects(
&self,
protocol_config,
metrics.clone(),
expensive_checks,
&certificate_deny_set,
&tx_info.executed_epoch,
tx_info.epoch_start_timestamp,
CheckedInputObjects::new_for_replay(input_objects.clone()),
tx_info.gas.clone(),
gas_status,
transaction_kind.clone(),
tx_info.sender,
*tx_digest,
);
let (inner_store, gas_status, effects, _timings, result) = executor
.execute_transaction_to_effects(
&self,
protocol_config,
metrics.clone(),
expensive_checks,
&certificate_deny_set,
&tx_info.executed_epoch,
tx_info.epoch_start_timestamp,
CheckedInputObjects::new_for_replay(input_objects.clone()),
tx_info.gas.clone(),
gas_status,
transaction_kind.clone(),
tx_info.sender,
*tx_digest,
);

if let Err(err) = self.pretty_print_for_tracing(
&gas_status,
Expand Down Expand Up @@ -924,7 +925,7 @@ impl LocalExec {
.unwrap();
let (kind, signer, gas) = executable.transaction_data().execution_parts();
let executor = sui_execution::executor(&protocol_config, true, None).unwrap();
let (_, _, effects, exec_res) = executor.execute_transaction_to_effects(
let (_, _, effects, _timings, exec_res) = executor.execute_transaction_to_effects(
&store,
&protocol_config,
Arc::new(LimitsMetrics::new(&Registry::new())),
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-single-node-benchmark/src/single_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ impl SingleValidator {
)
.unwrap();
let (kind, signer, gas) = executable.transaction_data().execution_parts();
let (inner_temp_store, _, effects, _) =
let (inner_temp_store, _, effects, _timings, _) =
self.epoch_store.executor().execute_transaction_to_effects(
&store,
self.epoch_store.protocol_config(),
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-swarm-config/src/network_config_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ mod test {
let (kind, signer, _) = transaction_data.execution_parts();
let input_objects = CheckedInputObjects::new_for_genesis(vec![]);

let (_inner_temp_store, _, effects, _execution_error) = executor
let (_inner_temp_store, _, effects, _timings, _execution_error) = executor
.execute_transaction_to_effects(
&InMemoryStorage::new(Vec::new()),
&protocol_config,
Expand Down
7 changes: 7 additions & 0 deletions crates/sui-types/src/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use move_core_types::language_storage::TypeTag;
use once_cell::sync::Lazy;
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::time::Duration;

/// A type containing all of the information needed to work with a deleted shared object in
/// execution and when committing the execution effects of the transaction. This holds:
Expand Down Expand Up @@ -170,6 +171,12 @@ impl ExecutionResultsV2 {
}
}

pub enum ExecutionTiming {
Success(Duration),
Abort(Duration),
}
pub type ResultWithTimings<R, E> = Result<(R, Vec<ExecutionTiming>), (E, Vec<ExecutionTiming>)>;

/// If a transaction digest shows up in this list, when executing such transaction,
/// we will always return `ExecutionError::CertificateDenied` without executing it (but still do
/// gas smashing). Because this list is not gated by protocol version, there are a few important
Expand Down
19 changes: 18 additions & 1 deletion crates/sui-types/src/sui_system_state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ pub mod advance_epoch_result_injection {
use crate::{
committee::EpochId,
error::{ExecutionError, ExecutionErrorKind},
execution::ResultWithTimings,
};
use std::cell::RefCell;

Expand All @@ -464,12 +465,28 @@ pub mod advance_epoch_result_injection {
/// This function is used to modify the result of advance_epoch transaction for testing.
/// If the override is set, the result will be an execution error, otherwise the original result will be returned.
pub fn maybe_modify_result(
result: ResultWithTimings<(), ExecutionError>,
current_epoch: EpochId,
) -> ResultWithTimings<(), ExecutionError> {
if let Some((start, end)) = OVERRIDE.with(|o| *o.borrow()) {
if current_epoch >= start && current_epoch < end {
return Err((
ExecutionError::new(ExecutionErrorKind::FunctionNotFound, None),
vec![],
));
}
}
result
}

// For old execution versions that don't report timings
pub fn maybe_modify_result_legacy(
result: Result<(), ExecutionError>,
current_epoch: EpochId,
) -> Result<(), ExecutionError> {
if let Some((start, end)) = OVERRIDE.with(|o| *o.borrow()) {
if current_epoch >= start && current_epoch < end {
return Err::<(), ExecutionError>(ExecutionError::new(
return Err(ExecutionError::new(
ExecutionErrorKind::FunctionNotFound,
None,
));
Expand Down
Loading

0 comments on commit 6210d3b

Please sign in to comment.