perf(persistence): improve write batch for HashedPostState & TrieUpdatesSorted

[duyquang6]

• Batch HashedPostState & TrieUpdatesSorted in save_blocks to improve batch processing performance.

Performance Results:

• Tested on devnet with 60k TPS raw-transfer workload, with default config (engine.persistence-thresh-hold = 3):
  • Before: ~50ms per block → ~150ms total for 3 blocks
  • After: ~50ms total for 3 blocks (3x improvement)
    Note: This assumes the number of accounts remains relatively stable across blocks, which aligns with typical production workloads.

Before:

*Note: p0.9 is sum of total time for all persist blocks, so it will be multiple by number of block need to persist

After:

Updated:

With --engine.persistence-threshold 6, so it need persist 7 blocks per interval:

For TrieUpdatesSorted:

after:

• reduce from 125ms -> ~100ms

Note: If we use default config: flush 3 blocks interval, the diff is not much different
But this still a win if in case our persistence very slow, and it need persist tons of block to catch-up with the tip

[shekhirin]

LGTM, cc @mediocregopher

[mediocregopher]

One nitpick on a comment, and a suggestion for another easy win you could do here but it's optional

[mediocregopher]

This comment isn't accurate, we can collect a TrieUpdatesSorted and extend it with the trie updates from each block, and then only do a single write_trie_updates_sorted at the end, just like you're doing here with the hashed state. Might be worth trying out in this PR?

[duyquang6]

hi @mediocregopher, about comment, that is obsolete I think

Let me try benchmark and update to you again

[duyquang6]

Thank for suggestion, here is bench result

I use --engine.persistence-threshold 6, so it need persist 7 blocks per interval:
before:

after:

reduce from 125ms -> ~100ms
Note: with default --engine.persistence-threshold 2: flush 3 blocks interval, the difference is not much

[mediocregopher]

Very nice, thanks for re-benching it 🙏

[mediocregopher]

@duyquang6 this had to be reverted as it caused state root mismatches.

The issue is that the writing of changesets (write_state for state, and write_trie_changesets for trie nodes) must be called with the updates for the previous block having been written, otherwise incorrect changesets get written. By aggregating updates and writing them at the end these method calls are not called on top of correct DB state.

write_trie_changesets accepts an overlay argument which I think could be used with the updates aggregated so far to correctly write changesets even while aggregating. write_state does not accept an argument like this, one would need to be added to achieve the aggregation logic.

[duyquang6]

@duyquang6 this had to be reverted as it caused state root mismatches.

The issue is that the writing of changesets (write_state for state, and write_trie_changesets for trie nodes) must be called with the updates for the previous block having been written, otherwise incorrect changesets get written. By aggregating updates and writing them at the end these method calls are not called on top of correct DB state.

write_trie_changesets accepts an overlay argument which I think could be used with the updates aggregated so far to correctly write changesets even while aggregating. write_state does not accept an argument like this, one would need to be added to achieve the aggregation logic.

Hi @mediocregopher,

I think the root cause is indeed that write_trie_changesets depends on write_trie_updates_sorted, as you mentioned.

Regarding the potential conflict between write_hashed_state and write_state, I believe it’s not relevant in this context. write_hashed_state only writes to:

• HashedAccounts
• HashedStorages

whereas write_state:

• read from: BlockBodyIndices, PlainAccountState, PlainStorageState
• write to: Receipts, PlainAccountState, PlainStorageState, StorageChangeSets, AccountChangeSets, Bytecodes

Given that they operate on separate data, batching write_hashed_state should be safe

[mediocregopher]

@duyquang6

Given that they operate on separate data, batching write_hashed_state should be safe

That makes sense, I guess it was only the trie changesets which caused the state root mismatches we saw then.

[duyquang6]

That makes sense, I guess it was only the trie changesets which caused the state root mismatches we saw then.

Hi sir, I'm planning to cherry-pick the write_hashed_state batching changes into a separate PR, I think that still a valuable improvement, since they're likely unrelated to the state root mismatch bug.

write_trie_changesets accepts an overlay argument which I think could be used with the updates aggregated so far to correctly write changesets even while aggregating

About this, I also had sometimes to play with this aggregate, but the bench result is not good, go one by one block still better in this step

I'll reference this discussion in the new PR