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: remove empty HashMap instances from TrieUpdates and HashedPostState #13976

Merged
merged 2 commits into from
Jan 28, 2025

Conversation

kien-rise
Copy link
Contributor

@kien-rise kien-rise commented Jan 24, 2025

This is the simplified version of #13929.

Checklist

Checklist

  • Do benchmarks
  • Improve PR description

Problem

In our benchmarks, we discovered that TrieUpdates::insert_storage_updates and HashedPostState::from_bundle_state are adding hundreds of thousands of empty HashMap instances. Since an empty HashMap does nothing, they can be safely removed without affecting correctness. By removing these empty HashMaps, we can reduce both memory usage (particularly heap allocation) and computation time.


To help visualize this better, here's an example of a state update from an ERC20 transaction:

{
  0x420000000000000000000000000000000000001b: Account { info: AccountInfo { balance: 0, nonce: 0, code_hash: 0xfa8c9db6c6cab7108dea276f4cd09d575674eb0852c0fa3187e59e98ef977998 }, storage: {}, status: AccountStatus(Touched) }, 
  0x420000000000000000000000000000000000001a: Account { info: AccountInfo { balance: 87755350474236426, nonce: 0, code_hash: 0xfa8c9db6c6cab7108dea276f4cd09d575674eb0852c0fa3187e59e98ef977998 }, storage: {}, status: AccountStatus(Touched) }, 
  0xd0f350b13465b5251bb03e4bbf9fa1dbc4a378f3: Account { info: AccountInfo { balance: 0, nonce: 1, code_hash: 0x9a412367fa4e1b709d142ec5c1cb547e716dd3368a69c5ecb373ac619d954cda }, storage: {6003198035184720553317537415064026399565423954743751102363488919614904056574: EvmStorageSlot { original_value: 300, present_value: 299, is_cold: false }, 81223867398230462435007348759598610405472546022605592074303692128879761985171: EvmStorageSlot { original_value: 300, present_value: 301, is_cold: false }}, status: AccountStatus(Touched) }, 
  0x4200000000000000000000000000000000000019: Account { info: AccountInfo { balance: 117754043519, nonce: 0, code_hash: 0xfa8c9db6c6cab7108dea276f4cd09d575674eb0852c0fa3187e59e98ef977998 }, storage: {}, status: AccountStatus(Touched) }, 
  0x4200000000000000000000000000000000000011: Account { info: AccountInfo { balance: 16822006217, nonce: 0, code_hash: 0xfa8c9db6c6cab7108dea276f4cd09d575674eb0852c0fa3187e59e98ef977998 }, storage: {}, status: AccountStatus(Touched) }, 
  0x31c8e9e5e7f5ecb0dbadd2c611d6c90d85a522a0: Account { info: AccountInfo { balance: 299999458782401680, nonce: 10, code_hash: 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470 }, storage: {}, status: AccountStatus(Touched) }
}

In the example above, even though 6 accounts are involved, only one (the ERC20 contract) actually has storage changes. The issue is that the other 5 accounts contribute 5 empty HashMaps, which are no-ops and unnecessary.

Solution

Eliminate empty HashMaps by simply not adding them.

Notable changes

# crates/trie/common/src/updates.rs

impl TrieUpdates {

    /// Insert storage updates for a given hashed address.
    pub fn insert_storage_updates(
        &mut self,
        hashed_address: B256,
        storage_updates: StorageTrieUpdates,
    ) {
+        if storage_updates.is_empty() {
+            return;
+        }
         let existing = self.storage_tries.insert(hashed_address, storage_updates);
         debug_assert!(existing.is_none());
     }

# crates/trie/trie/src/state.rs

impl HashedPostState {

    pub fn from_bundle_state<'a, KH: KeyHasher>(
        state: impl IntoParallelIterator<Item = (&'a Address, &'a BundleAccount)>,
    ) -> Self {

         let mut storages = HashMap::with_capacity_and_hasher(hashed.len(), Default::default());
         for (address, (account, storage)) in hashed {
             accounts.insert(address, account);
-            storages.insert(address, storage);
+            if !storage.is_empty() {
+                storages.insert(address, storage);
+            }
         }
         Self { accounts, storages }
     }

Results

Run time analysis

Total run time for state root calculation (3)

We benchmarked the "before" and "after" under the same condition (same machine, test, TPS, config). The results show a significant improvement across all three of our playbooks:

(unit: μs) Before After Ratio
ERC20 585111076 406979612 0.6955595761
Raw Transfer 323457659 179755982 0.55573265
Uniswap 1254878698 1114317335 0.8879880874

Branch statistics

For each added if statement, we tracked how often the condition evaluates to true or false. Here’s the summary:

Condition ERC20 Raw Transfer Uniswap
storage_updates.is_empty() == true 1.7058% 1.8344% 1.7931%
storage.is_empty() == true 99.9846% 99.9960% 99.5428%

The data above shows that while if storage_updates.is_empty() does not lead to significant changes, the if !storage.is_empty() { ... } in HashedPostState::from_bundle_state is highly effective.

E2E Benchmarks
"785bc168-8538d2e7-erc20-30600"
{"tps": 30601.0, "gps": 1056921700.4938775, "is_chain_lagged": false, "chain_lag_distance": 0}
[406979612, 441650528]
"785bc168-8538d2e7-erc20-35000"
{"tps": 35001.0, "gps": 1208892408.0443406, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-erc20-36000"
{"tps": 36001.0, "gps": 1243434143.3961585, "is_chain_lagged": false, "chain_lag_distance": 0}
"785bc168-8538d2e7-erc20-36400"
{"tps": 36401.0, "gps": 1257247094.8883495, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-erc20-36500"
{"tps": 36501.0, "gps": 1260693706.9232643, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-erc20-36600"
{"tps": 36601.0, "gps": 1264152458.967033, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-erc20-36700"
{"tps": 36701.0, "gps": 1267609469.7209303, "is_chain_lagged": true, "chain_lag_distance": 3}
"785bc168-8538d2e7-erc20-36800"
{"tps": 36801.0, "gps": 1271062700.5061426, "is_chain_lagged": true, "chain_lag_distance": 3}
"785bc168-8538d2e7-erc20-37200"
{"tps": 37201.0, "gps": 1284879652.7344913, "is_chain_lagged": true, "chain_lag_distance": 4}
"785bc168-8538d2e7-erc20-37600"
{"tps": 37601.0, "gps": 1298689498.6323714, "is_chain_lagged": true, "chain_lag_distance": 4}
"785bc168-8538d2e7-erc20-38000"
{"tps": 38001.0, "gps": 1312505549.7563453, "is_chain_lagged": true, "chain_lag_distance": 6}
"785bc168-8538d2e7-erc20-44000"
{"tps": 44001.0, "gps": 1519733106.229075, "is_chain_lagged": true, "chain_lag_distance": 40}
"785bc168-8538d2e7-raw-transfer-100000"
{"tps": 70917.98574821853, "gps": 1489301229.0783849, "is_chain_lagged": true, "chain_lag_distance": 2}
"785bc168-8538d2e7-raw-transfer-54600"
{"tps": 54512.001818181816, "gps": 1144775549.149091, "is_chain_lagged": false, "chain_lag_distance": 0}
[179755982, 194221704]
"785bc168-8538d2e7-raw-transfer-62000"
{"tps": 61380.149590163935, "gps": 1289006639.2868853, "is_chain_lagged": true, "chain_lag_distance": 2}
"785bc168-8538d2e7-raw-transfer-63000"
{"tps": 62550.83054393306, "gps": 1313590953.6401675, "is_chain_lagged": false, "chain_lag_distance": 0}
"785bc168-8538d2e7-raw-transfer-65000"
{"tps": 64196.991416309014, "gps": 1348160328.695279, "is_chain_lagged": false, "chain_lag_distance": 0}
"785bc168-8538d2e7-raw-transfer-69000"
{"tps": 68050.51818181819, "gps": 1429084397.3454545, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-raw-transfer-70000"
{"tps": 67990.69772727272, "gps": 1427828167.1454546, "is_chain_lagged": false, "chain_lag_distance": 0}
"785bc168-8538d2e7-raw-transfer-72000"
{"tps": 69915.03971962616, "gps": 1468239350.3831775, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-raw-transfer-76000"
{"tps": 68643.54377880185, "gps": 1441537944.4147465, "is_chain_lagged": true, "chain_lag_distance": 2}
"785bc168-8538d2e7-raw-transfer-78000"
{"tps": 69454.44755244756, "gps": 1458566913.5244756, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-raw-transfer-80000"
{"tps": 70477.3905882353, "gps": 1480048723.2705882, "is_chain_lagged": true, "chain_lag_distance": 3}
"785bc168-8538d2e7-raw-transfer-82000"
{"tps": 71027.36255924171, "gps": 1491598139.0331755, "is_chain_lagged": true, "chain_lag_distance": 3}
"785bc168-8538d2e7-raw-transfer-84000"
{"tps": 69952.23696682464, "gps": 1469020502.3033175, "is_chain_lagged": true, "chain_lag_distance": 3}
"785bc168-8538d2e7-uniswap-6330"
{"tps": 6331.0, "gps": 882453837.7864077, "is_chain_lagged": false, "chain_lag_distance": 1}
[1114317335, 1141570733]
"785bc168-8538d2e7-uniswap-6580"
{"tps": 6581.0, "gps": 917313166.4229926, "is_chain_lagged": false, "chain_lag_distance": 0}
"785bc168-8538d2e7-uniswap-6590"
{"tps": 6591.0, "gps": 918700006.3924413, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-uniswap-6600"
{"tps": 6601.0, "gps": 920100031.6195819, "is_chain_lagged": false, "chain_lag_distance": 1}
"785bc168-8538d2e7-uniswap-6610"
{"tps": 6611.0, "gps": 921483410.1694952, "is_chain_lagged": true, "chain_lag_distance": 3}
"785bc168-8538d2e7-uniswap-6620"
{"tps": 6621.0, "gps": 922882071.8124034, "is_chain_lagged": true, "chain_lag_distance": 2}
"785bc168-8538d2e7-uniswap-6700"
{"tps": 6701.0, "gps": 934018761.980563, "is_chain_lagged": true, "chain_lag_distance": 7}
"785bc168-8538d2e7-uniswap-6860"
{"tps": 6861.0, "gps": 956326845.1132205, "is_chain_lagged": true, "chain_lag_distance": 17}

@kien-rise kien-rise changed the title [WIP] perf: only perform insert_storage_updates if storage_updates is not empty [WIP] perf: get rid of empty (no-op) HashMap values in TrieUpdates and HashedPostState Jan 25, 2025
@kien-rise kien-rise changed the title [WIP] perf: get rid of empty (no-op) HashMap values in TrieUpdates and HashedPostState [WIP] perf: remove empty (no-op) HashMap values from TrieUpdates and HashedPostState Jan 25, 2025
@kien-rise kien-rise changed the title [WIP] perf: remove empty (no-op) HashMap values from TrieUpdates and HashedPostState perf: remove empty (no-op) HashMap values from TrieUpdates and HashedPostState Jan 27, 2025
@kien-rise kien-rise marked this pull request as ready for review January 27, 2025 20:49
@kien-rise kien-rise changed the title perf: remove empty (no-op) HashMap values from TrieUpdates and HashedPostState perf: remove empty HashMap instances from TrieUpdates and HashedPostState Jan 27, 2025
Copy link
Member

@rkrasiuk rkrasiuk left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

@mattsse mattsse added this pull request to the merge queue Jan 28, 2025
@mattsse mattsse added the C-perf A change motivated by improving speed, memory usage or disk footprint label Jan 28, 2025
Merged via the queue into paradigmxyz:main with commit e11e1f3 Jan 28, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-perf A change motivated by improving speed, memory usage or disk footprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants