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.

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

perf(EthBuiltPayload): Arc SealedBlock #12351

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 11 additions & 8 deletions crates/ethereum/engine-primitives/src/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use reth_primitives::{SealedBlock, Withdrawals};
use reth_rpc_types_compat::engine::payload::{
block_to_payload_v1, block_to_payload_v3, convert_block_to_payload_field_v2,
};
use std::convert::Infallible;
use std::{convert::Infallible, sync::Arc};

/// Contains the built payload.
///
Expand All @@ -25,7 +25,7 @@ pub struct EthBuiltPayload {
/// Identifier of the payload
pub(crate) id: PayloadId,
/// The built block
pub(crate) block: SealedBlock,
pub(crate) block: Arc<SealedBlock>,
/// Block execution data for the payload, if any.
pub(crate) executed_block: Option<ExecutedBlock>,
/// The fees of the block
Expand All @@ -45,7 +45,7 @@ impl EthBuiltPayload {
/// Caution: This does not set any [`BlobTransactionSidecar`].
pub const fn new(
id: PayloadId,
block: SealedBlock,
block: Arc<SealedBlock>,
fees: U256,
executed_block: Option<ExecutedBlock>,
requests: Option<Requests>,
Expand All @@ -59,7 +59,7 @@ impl EthBuiltPayload {
}

/// Returns the built block(sealed)
pub const fn block(&self) -> &SealedBlock {
pub fn block(&self) -> &SealedBlock {
&self.block
}

Expand Down Expand Up @@ -127,7 +127,7 @@ impl BuiltPayload for &EthBuiltPayload {
// V1 engine_getPayloadV1 response
impl From<EthBuiltPayload> for ExecutionPayloadV1 {
fn from(value: EthBuiltPayload) -> Self {
block_to_payload_v1(value.block)
block_to_payload_v1(Arc::unwrap_or_clone(value.block))
}
}

Expand All @@ -136,7 +136,10 @@ impl From<EthBuiltPayload> for ExecutionPayloadEnvelopeV2 {
fn from(value: EthBuiltPayload) -> Self {
let EthBuiltPayload { block, fees, .. } = value;

Self { block_value: fees, execution_payload: convert_block_to_payload_field_v2(block) }
Self {
block_value: fees,
execution_payload: convert_block_to_payload_field_v2(Arc::unwrap_or_clone(block)),
}
}
}

Expand All @@ -145,7 +148,7 @@ impl From<EthBuiltPayload> for ExecutionPayloadEnvelopeV3 {
let EthBuiltPayload { block, fees, sidecars, .. } = value;

Self {
execution_payload: block_to_payload_v3(block),
execution_payload: block_to_payload_v3(Arc::unwrap_or_clone(block)),
block_value: fees,
// From the engine API spec:
//
Expand All @@ -166,7 +169,7 @@ impl From<EthBuiltPayload> for ExecutionPayloadEnvelopeV4 {
let EthBuiltPayload { block, fees, sidecars, requests, .. } = value;

Self {
execution_payload: block_to_payload_v3(block),
execution_payload: block_to_payload_v3(Arc::unwrap_or_clone(block)),
block_value: fees,
// From the engine API spec:
//
Expand Down
4 changes: 2 additions & 2 deletions crates/ethereum/payload/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,12 @@ where
body: BlockBody { transactions: executed_txs, ommers: vec![], withdrawals },
};

let sealed_block = block.seal_slow();
let sealed_block = Arc::new(block.seal_slow());
debug!(target: "payload_builder", ?sealed_block, "sealed built block");

// create the executed block data
let executed = ExecutedBlock {
block: Arc::new(sealed_block.clone()),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal is to remove this potentially expensive clone.

block: sealed_block.clone(),
senders: Arc::new(executed_senders),
execution_output: Arc::new(execution_outcome),
hashed_state: Arc::new(hashed_state),
Expand Down
5 changes: 3 additions & 2 deletions crates/payload/builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
//! ```
//! use std::future::Future;
//! use std::pin::Pin;
//! use std::sync::Arc;
//! use std::task::{Context, Poll};
//! use alloy_primitives::U256;
//! use reth_payload_builder::{EthBuiltPayload, PayloadBuilderError, KeepPayloadJobAlive, EthPayloadBuilderAttributes, PayloadJob, PayloadJobGenerator, PayloadKind};
Expand Down Expand Up @@ -56,7 +57,7 @@
//!
//! fn best_payload(&self) -> Result<EthBuiltPayload, PayloadBuilderError> {
//! // NOTE: some fields are omitted here for brevity
//! let payload = Block {
//! let block = Block {
//! header: Header {
//! parent_hash: self.attributes.parent,
//! timestamp: self.attributes.timestamp,
Expand All @@ -65,7 +66,7 @@
//! },
//! ..Default::default()
//! };
//! let payload = EthBuiltPayload::new(self.attributes.id, payload.seal_slow(), U256::ZERO, None, None);
//! let payload = EthBuiltPayload::new(self.attributes.id, Arc::new(block.seal_slow()), U256::ZERO, None, None);
//! Ok(payload)
//! }
//!
Expand Down
3 changes: 2 additions & 1 deletion crates/payload/builder/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use reth_provider::CanonStateNotification;
use std::{
future::Future,
pin::Pin,
sync::Arc,
task::{Context, Poll},
};

Expand Down Expand Up @@ -86,7 +87,7 @@ impl PayloadJob for TestPayloadJob {
fn best_payload(&self) -> Result<EthBuiltPayload, PayloadBuilderError> {
Ok(EthBuiltPayload::new(
self.attr.payload_id(),
Block::default().seal_slow(),
Arc::new(Block::default().seal_slow()),
U256::ZERO,
Some(ExecutedBlock::default()),
Some(Default::default()),
Expand Down
Loading