Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf: improve debug_traceBlock performance #11979

Merged
merged 5 commits into from
Oct 26, 2024
Merged

perf: improve debug_traceBlock performance #11979

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8e0d77f
reuse inspector
lakshya-sky Oct 22, 2024
5285b84
fix lints
lakshya-sky Oct 22, 2024
ebbc8e1
change rest of the geth style traces
lakshya-sky Oct 22, 2024
6152647
fix suggestions
lakshya-sky Oct 24, 2024
fc563cb
docs: clarify fused inspector
mattsse Oct 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 46 additions & 23 deletions crates/rpc/rpc/src/debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ where
let mut results = Vec::with_capacity(transactions.len());
let mut db = CacheDB::new(StateProviderDatabase::new(state));
let mut transactions = transactions.into_iter().enumerate().peekable();
let mut inspector = None;
while let Some((index, tx)) = transactions.next() {
let tx_hash = tx.hash;

Expand All @@ -124,16 +125,19 @@ where
handler_cfg: cfg.handler_cfg,
};
let (result, state_changes) = this.trace_transaction(
opts.clone(),
&opts,
env,
&mut db,
Some(TransactionContext {
block_hash,
tx_hash: Some(tx_hash),
tx_index: Some(index),
}),
&mut inspector,
)?;

inspector = inspector.map(|insp| insp.fused());

results.push(TraceResult::Success { result, tx_hash: Some(tx_hash) });
if transactions.peek().is_some() {
// need to apply the state changes of this transaction before executing the
Expand Down Expand Up @@ -295,14 +299,15 @@ where
};

this.trace_transaction(
opts,
&opts,
env,
&mut db,
Some(TransactionContext {
block_hash: Some(block_hash),
tx_index: Some(index),
tx_hash: Some(tx.hash),
}),
&mut None,
)
.map(|(trace, _)| trace)
})
Expand Down Expand Up @@ -573,6 +578,7 @@ where
let Bundle { transactions, block_override } = bundle;

let block_overrides = block_override.map(Box::new);
let mut inspector = None;

let mut transactions = transactions.into_iter().peekable();
while let Some(tx) = transactions.next() {
Expand All @@ -588,8 +594,15 @@ where
overrides,
)?;

let (trace, state) =
this.trace_transaction(tracing_options.clone(), env, &mut db, None)?;
let (trace, state) = this.trace_transaction(
&tracing_options,
env,
&mut db,
None,
&mut inspector,
)?;

inspector = inspector.map(|insp| insp.fused());

// If there is more transactions, commit the database
// If there is no transactions, but more bundles, commit to the database too
Expand Down Expand Up @@ -692,17 +705,24 @@ where

/// Executes the configured transaction with the environment on the given database.
///
/// It optionally takes shared inspector to avoid re-creating the inspector for each
///
/// transaction. This is useful when tracing multiple transactions in a block.
///
/// If the inspector is provided then `opts.tracer_config` is ignored.
///
/// Returns the trace frame and the state that got updated after executing the transaction.
///
/// Note: this does not apply any state overrides if they're configured in the `opts`.
///
/// Caution: this is blocking and should be performed on a blocking task.
fn trace_transaction(
&self,
opts: GethDebugTracingOptions,
opts: &GethDebugTracingOptions,
env: EnvWithHandlerCfg,
db: &mut StateCacheDb<'_>,
transaction_context: Option<TransactionContext>,
shared_inspector: &mut Option<TracingInspector>,
mattsse marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<(GethTrace, revm_primitives::EvmState), Eth::Error> {
let GethDebugTracingOptions { config, tracer, tracer_config, .. } = opts;

Expand All @@ -716,35 +736,38 @@ where
}
GethDebugBuiltInTracerType::CallTracer => {
let call_config = tracer_config
.clone()
.into_call_config()
.map_err(|_| EthApiError::InvalidTracerConfig)?;

let mut inspector = TracingInspector::new(
let mut inspector = shared_inspector.get_or_insert(TracingInspector::new(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also slightly redundant work if the inspector is present, we have this a few time and we can definitely solve this more elegantly

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah turns out we can't easily do this because we need the config

TracingInspectorConfig::from_geth_call_config(&call_config),
);
));

let (res, env) = self.eth_api().inspect(db, env, &mut inspector)?;

inspector.set_transaction_gas_limit(env.tx.gas_limit);

let frame = inspector
.with_transaction_gas_limit(env.tx.gas_limit)
.into_geth_builder()
.geth_builder()
.geth_call_traces(call_config, res.result.gas_used());

return Ok((frame.into(), res.state))
}
GethDebugBuiltInTracerType::PreStateTracer => {
let prestate_config = tracer_config
.clone()
.into_pre_state_config()
.map_err(|_| EthApiError::InvalidTracerConfig)?;

let mut inspector = TracingInspector::new(
let mut inspector = shared_inspector.get_or_insert(TracingInspector::new(
TracingInspectorConfig::from_geth_prestate_config(&prestate_config),
);
));
let (res, env) = self.eth_api().inspect(&mut *db, env, &mut inspector)?;

inspector.set_transaction_gas_limit(env.tx.gas_limit);
let frame = inspector
.with_transaction_gas_limit(env.tx.gas_limit)
.into_geth_builder()
.geth_builder()
.geth_prestate_traces(&res, &prestate_config, db)
.map_err(Eth::Error::from_eth_err)?;

Expand All @@ -755,6 +778,7 @@ where
}
GethDebugBuiltInTracerType::MuxTracer => {
let mux_config = tracer_config
.clone()
.into_mux_config()
.map_err(|_| EthApiError::InvalidTracerConfig)?;

Expand All @@ -769,6 +793,7 @@ where
}
GethDebugBuiltInTracerType::FlatCallTracer => {
let flat_call_config = tracer_config
.clone()
.into_flat_call_config()
.map_err(|_| EthApiError::InvalidTracerConfig)?;

Expand Down Expand Up @@ -799,10 +824,10 @@ where
}
#[cfg(feature = "js-tracer")]
GethDebugTracerType::JsTracer(code) => {
let config = tracer_config.into_json();
let config = tracer_config.clone().into_json();
let mut inspector =
revm_inspectors::tracing::js::JsInspector::with_transaction_context(
code,
code.clone(),
config,
transaction_context.unwrap_or_default(),
)
Expand All @@ -818,17 +843,15 @@ where
}

// default structlog tracer
let inspector_config = TracingInspectorConfig::from_geth_config(&config);

let mut inspector = TracingInspector::new(inspector_config);

let mut inspector = shared_inspector.get_or_insert_with(|| {
let inspector_config = TracingInspectorConfig::from_geth_config(config);
TracingInspector::new(inspector_config)
});
let (res, env) = self.eth_api().inspect(db, env, &mut inspector)?;
let gas_used = res.result.gas_used();
let return_value = res.result.into_output().unwrap_or_default();
let frame = inspector
.with_transaction_gas_limit(env.tx.gas_limit)
.into_geth_builder()
.geth_traces(gas_used, return_value, config);
inspector.set_transaction_gas_limit(env.tx.gas_limit);
let frame = inspector.geth_builder().geth_traces(gas_used, return_value, *config);

Ok((frame.into(), res.state))
}
Expand Down
Loading