Skip to content

Commit

Permalink
perf: query accounts with &Address to avoid copying address (#13554)
Browse files Browse the repository at this point in the history
  • Loading branch information
hai-rise authored Dec 25, 2024
1 parent 14c1c0b commit 031f430
Show file tree
Hide file tree
Showing 27 changed files with 74 additions and 71 deletions.
2 changes: 1 addition & 1 deletion crates/chain-state/src/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ mod tests {
}

impl AccountReader for MockStateProvider {
fn basic_account(&self, _address: Address) -> ProviderResult<Option<Account>> {
fn basic_account(&self, _address: &Address) -> ProviderResult<Option<Account>> {
Ok(None)
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/chain-state/src/memory_overlay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ impl<N: NodePrimitives> BlockHashReader for MemoryOverlayStateProviderRef<'_, N>
}

impl<N: NodePrimitives> AccountReader for MemoryOverlayStateProviderRef<'_, N> {
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
for block in &self.in_memory {
if let Some(account) = block.execution_output.account(&address) {
if let Some(account) = block.execution_output.account(address) {
return Ok(account);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/consensus/common/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ mod tests {
}

impl AccountReader for Provider {
fn basic_account(&self, _address: Address) -> ProviderResult<Option<Account>> {
fn basic_account(&self, _address: &Address) -> ProviderResult<Option<Account>> {
Ok(self.account)
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/engine/tree/benches/state_root_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ fn setup_provider(
// other updates
let should_process = match account.status {
AccountStatus::SelfDestructed => {
provider_rw.basic_account(*address).ok().flatten().is_some()
provider_rw.basic_account(address).ok().flatten().is_some()
}
_ => true,
};
Expand Down
6 changes: 3 additions & 3 deletions crates/revm/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub trait EvmStateProvider: Send + Sync {
/// Get basic account information.
///
/// Returns [`None`] if the account doesn't exist.
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>>;
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>>;

/// Get the hash of the block with the given number. Returns [`None`] if no block with this
/// number exists.
Expand All @@ -38,7 +38,7 @@ pub trait EvmStateProvider: Send + Sync {

// Blanket implementation of EvmStateProvider for any type that implements StateProvider.
impl<T: reth_storage_api::StateProvider> EvmStateProvider for T {
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
<T as reth_storage_api::AccountReader>::basic_account(self, address)
}

Expand Down Expand Up @@ -141,7 +141,7 @@ impl<DB: EvmStateProvider> DatabaseRef for StateProviderDatabase<DB> {
/// Returns `Ok` with `Some(AccountInfo)` if the account exists,
/// `None` if it doesn't, or an error if encountered.
fn basic_ref(&self, address: Address) -> Result<Option<AccountInfo>, Self::Error> {
Ok(self.basic_account(address)?.map(Into::into))
Ok(self.basic_account(&address)?.map(Into::into))
}

/// Retrieves the bytecode associated with a given code hash.
Expand Down
4 changes: 2 additions & 2 deletions crates/revm/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ impl StateProviderTest {
}

impl AccountReader for StateProviderTest {
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
Ok(self.accounts.get(&address).map(|(_, acc)| *acc))
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
Ok(self.accounts.get(address).map(|(_, acc)| *acc))
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-eth-api/src/helpers/estimate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub trait EstimateCall: Call {
// Optimize for simple transfer transactions, potentially reducing the gas estimate.
if env.tx.data.is_empty() {
if let TransactTo::Call(to) = env.tx.transact_to {
if let Ok(code) = db.db.account_code(to) {
if let Ok(code) = db.db.account_code(&to) {
let no_code_callee = code.map(|code| code.is_empty()).unwrap_or(true);
if no_code_callee {
// If the tx is a simple transfer (call to an account with no code) we can
Expand Down
10 changes: 5 additions & 5 deletions crates/rpc/rpc-eth-api/src/helpers/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub trait EthState: LoadState + SpawnBlocking {
self.spawn_blocking_io(move |this| {
Ok(this
.state_at_block_id_or_latest(block_id)?
.account_balance(address)
.account_balance(&address)
.map_err(Self::Error::from_eth_err)?
.unwrap_or_default())
})
Expand Down Expand Up @@ -131,7 +131,7 @@ pub trait EthState: LoadState + SpawnBlocking {
) -> impl Future<Output = Result<Option<Account>, Self::Error>> + Send {
self.spawn_blocking_io(move |this| {
let state = this.state_at_block_id(block_id)?;
let account = state.basic_account(address).map_err(Self::Error::from_eth_err)?;
let account = state.basic_account(&address).map_err(Self::Error::from_eth_err)?;
let Some(account) = account else { return Ok(None) };

// Check whether the distance to the block exceeds the maximum configured proof window.
Expand Down Expand Up @@ -252,7 +252,7 @@ pub trait LoadState:
// first fetch the on chain nonce of the account
let on_chain_account_nonce = this
.latest_state()?
.account_nonce(address)
.account_nonce(&address)
.map_err(Self::Error::from_eth_err)?
.unwrap_or_default();

Expand Down Expand Up @@ -290,7 +290,7 @@ pub trait LoadState:
// first fetch the on chain nonce of the account
let on_chain_account_nonce = this
.state_at_block_id_or_latest(block_id)?
.account_nonce(address)
.account_nonce(&address)
.map_err(Self::Error::from_eth_err)?
.unwrap_or_default();

Expand Down Expand Up @@ -333,7 +333,7 @@ pub trait LoadState:
self.spawn_blocking_io(move |this| {
Ok(this
.state_at_block_id_or_latest(block_id)?
.account_code(address)
.account_code(&address)
.map_err(Self::Error::from_eth_err)?
.unwrap_or_default()
.original_bytes())
Expand Down
8 changes: 4 additions & 4 deletions crates/rpc/rpc-eth-types/src/cache/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl reth_storage_api::StateProofProvider for StateProviderTraitObjWrapper<'_> {
impl reth_storage_api::AccountReader for StateProviderTraitObjWrapper<'_> {
fn basic_account(
&self,
address: revm_primitives::Address,
address: &revm_primitives::Address,
) -> reth_errors::ProviderResult<Option<reth_primitives::Account>> {
self.0.basic_account(address)
}
Expand Down Expand Up @@ -163,21 +163,21 @@ impl StateProvider for StateProviderTraitObjWrapper<'_> {

fn account_code(
&self,
addr: revm_primitives::Address,
addr: &revm_primitives::Address,
) -> reth_errors::ProviderResult<Option<reth_primitives::Bytecode>> {
self.0.account_code(addr)
}

fn account_balance(
&self,
addr: revm_primitives::Address,
addr: &revm_primitives::Address,
) -> reth_errors::ProviderResult<Option<U256>> {
self.0.account_balance(addr)
}

fn account_nonce(
&self,
addr: revm_primitives::Address,
addr: &revm_primitives::Address,
) -> reth_errors::ProviderResult<Option<u64>> {
self.0.account_nonce(addr)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc/src/reth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ where
let hash_map = accounts_before.iter().try_fold(
HashMap::default(),
|mut hash_map, account_before| -> RethResult<_> {
let current_balance = state.account_balance(account_before.address)?;
let current_balance = state.account_balance(&account_before.address)?;
let prev_balance = account_before.info.map(|info| info.balance);
if current_balance != prev_balance {
hash_map.insert(account_before.address, current_balance.unwrap_or_default());
Expand Down
14 changes: 7 additions & 7 deletions crates/stages/stages/src/stages/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -939,17 +939,17 @@ mod tests {

// assert accounts
assert_eq!(
provider.basic_account(account1),
provider.basic_account(&account1),
Ok(Some(account1_info)),
"Post changed of a account"
);
assert_eq!(
provider.basic_account(account2),
provider.basic_account(&account2),
Ok(Some(account2_info)),
"Post changed of a account"
);
assert_eq!(
provider.basic_account(account3),
provider.basic_account(&account3),
Ok(Some(account3_info)),
"Post changed of a account"
);
Expand Down Expand Up @@ -1075,19 +1075,19 @@ mod tests {

// assert unwind stage
assert_eq!(
provider.basic_account(acc1),
provider.basic_account(&acc1),
Ok(Some(acc1_info)),
"Pre changed of a account"
);
assert_eq!(
provider.basic_account(acc2),
provider.basic_account(&acc2),
Ok(Some(acc2_info)),
"Post changed of a account"
);

let miner_acc = address!("2adc25665018aa1fe0e6bc666dac8fc2697ff9ba");
assert_eq!(
provider.basic_account(miner_acc),
provider.basic_account(&miner_acc),
Ok(None),
"Third account should be unwound"
);
Expand Down Expand Up @@ -1173,7 +1173,7 @@ mod tests {

// assert unwind stage
let provider = test_db.factory.database_provider_rw().unwrap();
assert_eq!(provider.basic_account(destroyed_address), Ok(None), "Account was destroyed");
assert_eq!(provider.basic_account(&destroyed_address), Ok(None), "Account was destroyed");

assert_eq!(
provider.tx_ref().get::<tables::PlainStorageState>(destroyed_address),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ impl<N: ProviderNodeTypes> ChangeSetReader for BlockchainProvider2<N> {

impl<N: ProviderNodeTypes> AccountReader for BlockchainProvider2<N> {
/// Get basic account information.
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
self.consistent_provider()?.basic_account(address)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ impl<SP: StateProvider, EDP: ExecutionDataProvider> BlockHashReader
}

impl<SP: StateProvider, EDP: ExecutionDataProvider> AccountReader for BundleStateProvider<SP, EDP> {
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
if let Some(account) =
self.block_execution_data_provider.execution_outcome().account(&address)
self.block_execution_data_provider.execution_outcome().account(address)
{
Ok(account)
} else {
Expand Down
6 changes: 3 additions & 3 deletions crates/storage/provider/src/providers/consistent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl<N: ProviderNodeTypes> ConsistentProvider<N> {
let AccountBeforeTx { info: old_info, address } = account_before;
match state.entry(address) {
hash_map::Entry::Vacant(entry) => {
let new_info = state_provider.basic_account(address)?;
let new_info = state_provider.basic_account(&address)?;
entry.insert((old_info, new_info, HashMap::new()));
}
hash_map::Entry::Occupied(mut entry) => {
Expand All @@ -252,7 +252,7 @@ impl<N: ProviderNodeTypes> ConsistentProvider<N> {
// get account state or insert from plain state.
let account_state = match state.entry(address) {
hash_map::Entry::Vacant(entry) => {
let present_info = state_provider.basic_account(address)?;
let present_info = state_provider.basic_account(&address)?;
entry.insert((present_info, present_info, HashMap::new()))
}
hash_map::Entry::Occupied(entry) => entry.into_mut(),
Expand Down Expand Up @@ -1441,7 +1441,7 @@ impl<N: ProviderNodeTypes> ChangeSetReader for ConsistentProvider<N> {

impl<N: ProviderNodeTypes> AccountReader for ConsistentProvider<N> {
/// Get basic account information.
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
// use latest state provider
let state_provider = self.latest_ref()?;
state_provider.basic_account(address)
Expand Down
4 changes: 2 additions & 2 deletions crates/storage/provider/src/providers/database/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,8 +862,8 @@ impl<TX: DbTxMut + DbTx + 'static, N: NodeTypes> DatabaseProvider<TX, N> {
}

impl<TX: DbTx, N: NodeTypes> AccountReader for DatabaseProvider<TX, N> {
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
Ok(self.tx.get::<tables::PlainAccountState>(address)?)
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
Ok(self.tx.get_by_encoded_key::<tables::PlainAccountState>(address)?)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/storage/provider/src/providers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ impl<N: ProviderNodeTypes> ChangeSetReader for BlockchainProvider<N> {

impl<N: ProviderNodeTypes> AccountReader for BlockchainProvider<N> {
/// Get basic account information.
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
self.database.provider()?.basic_account(address)
}
}
Expand Down
37 changes: 20 additions & 17 deletions crates/storage/provider/src/providers/state/historical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,21 +249,21 @@ impl<Provider: DBProvider + BlockNumReader + StateCommitmentProvider> AccountRea
for HistoricalStateProviderRef<'_, Provider>
{
/// Get basic account information.
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
match self.account_history_lookup(address)? {
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
match self.account_history_lookup(*address)? {
HistoryInfo::NotYetWritten => Ok(None),
HistoryInfo::InChangeset(changeset_block_number) => Ok(self
.tx()
.cursor_dup_read::<tables::AccountChangeSets>()?
.seek_by_key_subkey(changeset_block_number, address)?
.filter(|acc| acc.address == address)
.seek_by_key_subkey(changeset_block_number, *address)?
.filter(|acc| &acc.address == address)
.ok_or(ProviderError::AccountChangesetNotFound {
block_number: changeset_block_number,
address,
address: *address,
})?
.info),
HistoryInfo::InPlainState | HistoryInfo::MaybeInPlainState => {
Ok(self.tx().get::<tables::PlainAccountState>(address)?)
Ok(self.tx().get_by_encoded_key::<tables::PlainAccountState>(address)?)
}
}
}
Expand Down Expand Up @@ -633,43 +633,46 @@ mod tests {
let db = factory.provider().unwrap();

// run
assert_eq!(HistoricalStateProviderRef::new(&db, 1).basic_account(ADDRESS), Ok(None));
assert_eq!(HistoricalStateProviderRef::new(&db, 1).basic_account(&ADDRESS), Ok(None));
assert_eq!(
HistoricalStateProviderRef::new(&db, 2).basic_account(ADDRESS),
HistoricalStateProviderRef::new(&db, 2).basic_account(&ADDRESS),
Ok(Some(acc_at3))
);
assert_eq!(
HistoricalStateProviderRef::new(&db, 3).basic_account(ADDRESS),
HistoricalStateProviderRef::new(&db, 3).basic_account(&ADDRESS),
Ok(Some(acc_at3))
);
assert_eq!(
HistoricalStateProviderRef::new(&db, 4).basic_account(ADDRESS),
HistoricalStateProviderRef::new(&db, 4).basic_account(&ADDRESS),
Ok(Some(acc_at7))
);
assert_eq!(
HistoricalStateProviderRef::new(&db, 7).basic_account(ADDRESS),
HistoricalStateProviderRef::new(&db, 7).basic_account(&ADDRESS),
Ok(Some(acc_at7))
);
assert_eq!(
HistoricalStateProviderRef::new(&db, 9).basic_account(ADDRESS),
HistoricalStateProviderRef::new(&db, 9).basic_account(&ADDRESS),
Ok(Some(acc_at10))
);
assert_eq!(
HistoricalStateProviderRef::new(&db, 10).basic_account(ADDRESS),
HistoricalStateProviderRef::new(&db, 10).basic_account(&ADDRESS),
Ok(Some(acc_at10))
);
assert_eq!(
HistoricalStateProviderRef::new(&db, 11).basic_account(ADDRESS),
HistoricalStateProviderRef::new(&db, 11).basic_account(&ADDRESS),
Ok(Some(acc_at15))
);
assert_eq!(
HistoricalStateProviderRef::new(&db, 16).basic_account(ADDRESS),
HistoricalStateProviderRef::new(&db, 16).basic_account(&ADDRESS),
Ok(Some(acc_plain))
);

assert_eq!(HistoricalStateProviderRef::new(&db, 1).basic_account(HIGHER_ADDRESS), Ok(None));
assert_eq!(
HistoricalStateProviderRef::new(&db, 1000).basic_account(HIGHER_ADDRESS),
HistoricalStateProviderRef::new(&db, 1).basic_account(&HIGHER_ADDRESS),
Ok(None)
);
assert_eq!(
HistoricalStateProviderRef::new(&db, 1000).basic_account(&HIGHER_ADDRESS),
Ok(Some(higher_acc_plain))
);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/storage/provider/src/providers/state/latest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ impl<'b, Provider: DBProvider> LatestStateProviderRef<'b, Provider> {

impl<Provider: DBProvider> AccountReader for LatestStateProviderRef<'_, Provider> {
/// Get basic account information.
fn basic_account(&self, address: Address) -> ProviderResult<Option<Account>> {
self.tx().get::<tables::PlainAccountState>(address).map_err(Into::into)
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
self.tx().get_by_encoded_key::<tables::PlainAccountState>(address).map_err(Into::into)
}
}

Expand Down
Loading

0 comments on commit 031f430

Please sign in to comment.