Skip to content

Conversation

rodrigo-o
Copy link
Collaborator

@rodrigo-o rodrigo-o commented Oct 13, 2025

Motivation

This PR reduces memory by 40~50%

Description

This PR sits is basically #4839 but instead of starting from #4821 it starts from main, and after different testing scenarios we came up with this cut in memory:

The caveat is that it appears to be 50% slower, we need to make specific measures to validate the results seen in #4839 (no longer the case, see #4853 (comment))

Copy link

github-actions bot commented Oct 13, 2025

Lines of code report

Total lines added: 0
Total lines removed: 12
Total lines changed: 12

Detailed view
+-------------------------------------------+-------+------+
| File                                      | Lines | Diff |
+-------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/rocksdb.rs | 1238  | -12  |
+-------------------------------------------+-------+------+

@rodrigo-o
Copy link
Collaborator Author

rodrigo-o commented Oct 13, 2025

Here we have some initial measurements for backing the 40~50% of memory reduction mentioned in this PR. We are right now testing it again without storage_chunk: #4839 (comment). Here is an initial measurement for the first ~3hours in mainnet:

Node Memory (RSS) & Host memory in mages

Main This PR
Mean: ~16GB & Max: ~22GB Mean: ~8GB & Max: ~15GB
image image

@rodrigo-o rodrigo-o marked this pull request as ready for review October 13, 2025 19:11
@rodrigo-o rodrigo-o requested a review from a team as a code owner October 13, 2025 19:11
@Copilot Copilot AI review requested due to automatic review settings October 13, 2025 19:11
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Oct 13, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reverts optimistic transactions in RocksDB and removes explicit cache configurations to reduce memory usage by 40-50%. The changes replace OptimisticTransactionDB with standard DBWithThreadMode and remove cache-related options from RocksDB configuration.

  • Replace OptimisticTransactionDB with DBWithThreadMode across all RocksDB implementations
  • Remove explicit cache setup and cache-related block options to reduce memory footprint
  • Update batch operations to use standard WriteBatch instead of WriteBatchWithTransaction

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
crates/storage/trie_db/rocksdb_locked.rs Updated type signatures to use DBWithThreadMode instead of OptimisticTransactionDB
crates/storage/trie_db/rocksdb.rs Replaced optimistic transaction DB with standard DB and updated batch operations
crates/storage/store_db/rocksdb.rs Removed cache configuration and replaced optimistic transactions with standard DB operations
CHANGELOG.md Added entry documenting the memory optimization changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

github-actions bot commented Oct 13, 2025

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 89.080 ± 0.128 88.787 89.230 1.00
head 89.139 ± 0.167 88.860 89.322 1.00 ± 0.00

@rodrigo-o rodrigo-o marked this pull request as draft October 14, 2025 13:58
@rodrigo-o
Copy link
Collaborator Author

rodrigo-o commented Oct 14, 2025

We are going to make this changes behind a low-memory flag to avoid the snap sync time regression when we prefer a quicker syncing.

After pathbased, this PR doesn't reduce performance as measured before. Right now snapsync takes the same time with and without optimistic transactions.

@rodrigo-o rodrigo-o marked this pull request as ready for review October 17, 2025 18:29
@jrchatruc jrchatruc enabled auto-merge October 17, 2025 20:28
@jrchatruc jrchatruc added this pull request to the merge queue Oct 17, 2025
Merged via the queue into main with commit f81aedd Oct 17, 2025
31 checks passed
@jrchatruc jrchatruc deleted the perf/rocksdb-config-memory-enhancement branch October 17, 2025 21:18
@github-project-automation github-project-automation bot moved this from Todo to Done in ethrex_performance Oct 17, 2025
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Oct 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client performance

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants