From d3bc17be874ce5542f20129110f25f98e57b4d1e Mon Sep 17 00:00:00 2001 From: Hai NguyenQuang Date: Sat, 13 Sep 2025 15:38:18 +0700 Subject: [PATCH] perf(db): open MDBX DBIs only once at startup --- Cargo.lock | 2 - crates/storage/db/Cargo.toml | 2 - .../storage/db/src/implementation/mdbx/mod.rs | 29 ++++++++++---- .../storage/db/src/implementation/mdbx/tx.rs | 39 ++++++------------- crates/storage/db/src/mdbx.rs | 2 +- 5 files changed, 35 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 96bd4a3e9f6..332ff1f2c06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2746,7 +2746,6 @@ version = "6.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5041cc499144891f3790297212f32a74fb938e5136a14943f338ef9e0ae276cf" dependencies = [ - "arbitrary", "cfg-if", "crossbeam-utils", "hashbrown 0.14.5", @@ -7608,7 +7607,6 @@ dependencies = [ "arbitrary", "assert_matches", "codspeed-criterion-compat", - "dashmap 6.1.0", "derive_more", "eyre", "metrics", diff --git a/crates/storage/db/Cargo.toml b/crates/storage/db/Cargo.toml index 264a1f1f628..5e2c2f31b9b 100644 --- a/crates/storage/db/Cargo.toml +++ b/crates/storage/db/Cargo.toml @@ -39,7 +39,6 @@ derive_more.workspace = true rustc-hash = { workspace = true, optional = true, features = ["std"] } sysinfo = { workspace = true, features = ["system"] } parking_lot = { workspace = true, optional = true } -dashmap.workspace = true # arbitrary utils strum = { workspace = true, features = ["derive"], optional = true } @@ -92,7 +91,6 @@ arbitrary = [ "alloy-consensus/arbitrary", "reth-primitives-traits/arbitrary", "reth-prune-types/arbitrary", - "dashmap/arbitrary", ] op = [ "reth-db-api/op", diff --git a/crates/storage/db/src/implementation/mdbx/mod.rs b/crates/storage/db/src/implementation/mdbx/mod.rs index faa784de698..d2d5b295a63 100644 --- a/crates/storage/db/src/implementation/mdbx/mod.rs +++ b/crates/storage/db/src/implementation/mdbx/mod.rs @@ -23,6 +23,7 @@ use reth_libmdbx::{ use reth_storage_errors::db::LogLevel; use reth_tracing::tracing::error; use std::{ + collections::HashMap, ops::{Deref, Range}, path::Path, sync::Arc, @@ -190,6 +191,12 @@ impl DatabaseArguments { pub struct DatabaseEnv { /// Libmdbx-sys environment. inner: Environment, + /// Opened DBIs for reuse. + /// Important: Do not manually close these DBIs, like via `mdbx_dbi_close`. + /// More generally, do not dynamically create, re-open, or drop tables at + /// runtime. It's better to perform table creation and migration only once + /// at startup. + dbis: Arc>, /// Cache for metric handles. If `None`, metrics are not recorded. metrics: Option>, /// Write lock for when dealing with a read-write environment. @@ -201,16 +208,18 @@ impl Database for DatabaseEnv { type TXMut = tx::Tx; fn tx(&self) -> Result { - Tx::new_with_metrics( + Tx::new( self.inner.begin_ro_txn().map_err(|e| DatabaseError::InitTx(e.into()))?, + self.dbis.clone(), self.metrics.clone(), ) .map_err(|e| DatabaseError::InitTx(e.into())) } fn tx_mut(&self) -> Result { - Tx::new_with_metrics( + Tx::new( self.inner.begin_rw_txn().map_err(|e| DatabaseError::InitTx(e.into()))?, + self.dbis.clone(), self.metrics.clone(), ) .map_err(|e| DatabaseError::InitTx(e.into())) @@ -445,6 +454,7 @@ impl DatabaseEnv { let env = Self { inner: inner_env.open(path).map_err(|e| DatabaseError::Open(e.into()))?, + dbis: Arc::default(), metrics: None, _lock_file, }; @@ -459,23 +469,27 @@ impl DatabaseEnv { } /// Creates all the tables defined in [`Tables`], if necessary. - pub fn create_tables(&self) -> Result<(), DatabaseError> { + pub fn create_tables(&mut self) -> Result<(), DatabaseError> { self.create_tables_for::() } /// Creates all the tables defined in the given [`TableSet`], if necessary. - pub fn create_tables_for(&self) -> Result<(), DatabaseError> { + pub fn create_tables_for(&mut self) -> Result<(), DatabaseError> { let tx = self.inner.begin_rw_txn().map_err(|e| DatabaseError::InitTx(e.into()))?; + let mut dbis = HashMap::with_capacity(Tables::ALL.len()); for table in TS::tables() { let flags = if table.is_dupsort() { DatabaseFlags::DUP_SORT } else { DatabaseFlags::default() }; - tx.create_db(Some(table.name()), flags) + let db = tx + .create_db(Some(table.name()), flags) .map_err(|e| DatabaseError::CreateTable(e.into()))?; + dbis.insert(table.name(), db.dbi()); } tx.commit().map_err(|e| DatabaseError::Commit(e.into()))?; + self.dbis = Arc::new(dbis); Ok(()) } @@ -543,8 +557,9 @@ mod tests { /// Create database for testing with specified path fn create_test_db_with_path(kind: DatabaseEnvKind, path: &Path) -> DatabaseEnv { - let env = DatabaseEnv::open(path, kind, DatabaseArguments::new(ClientVersion::default())) - .expect(ERROR_DB_CREATION); + let mut env = + DatabaseEnv::open(path, kind, DatabaseArguments::new(ClientVersion::default())) + .expect(ERROR_DB_CREATION); env.create_tables().expect(ERROR_TABLE_CREATION); env } diff --git a/crates/storage/db/src/implementation/mdbx/tx.rs b/crates/storage/db/src/implementation/mdbx/tx.rs index 04aa4f8f85c..1d3e3124306 100644 --- a/crates/storage/db/src/implementation/mdbx/tx.rs +++ b/crates/storage/db/src/implementation/mdbx/tx.rs @@ -5,7 +5,6 @@ use crate::{ metrics::{DatabaseEnvMetrics, Operation, TransactionMode, TransactionOutcome}, DatabaseError, }; -use dashmap::DashMap; use reth_db_api::{ table::{Compress, DupSort, Encode, Table, TableImporter}, transaction::{DbTx, DbTxMut}, @@ -15,6 +14,7 @@ use reth_storage_errors::db::{DatabaseWriteError, DatabaseWriteOperation}; use reth_tracing::tracing::{debug, trace, warn}; use std::{ backtrace::Backtrace, + collections::HashMap, marker::PhantomData, sync::{ atomic::{AtomicBool, Ordering}, @@ -33,8 +33,7 @@ pub struct Tx { pub inner: Transaction, /// Cached MDBX DBIs for reuse. - /// TODO: Reuse DBIs even among transactions, ideally with no synchronization overhead. - dbis: DashMap<&'static str, MDBX_dbi>, + dbis: Arc>, /// Handler for metrics with its own [Drop] implementation for cases when the transaction isn't /// closed by [`Tx::commit`] or [`Tx::abort`], but we still need to report it in the metrics. @@ -44,17 +43,12 @@ pub struct Tx { } impl Tx { - /// Creates new `Tx` object with a `RO` or `RW` transaction. - #[inline] - pub fn new(inner: Transaction) -> Self { - Self::new_inner(inner, None) - } - /// Creates new `Tx` object with a `RO` or `RW` transaction and optionally enables metrics. #[inline] #[track_caller] - pub(crate) fn new_with_metrics( + pub(crate) fn new( inner: Transaction, + dbis: Arc>, env_metrics: Option>, ) -> reth_libmdbx::Result { let metrics_handler = env_metrics @@ -65,12 +59,7 @@ impl Tx { Ok(handler) }) .transpose()?; - Ok(Self::new_inner(inner, metrics_handler)) - } - - #[inline] - fn new_inner(inner: Transaction, metrics_handler: Option>) -> Self { - Self { inner, metrics_handler, dbis: DashMap::new() } + Ok(Self { inner, dbis, metrics_handler }) } /// Gets this transaction ID. @@ -80,17 +69,13 @@ impl Tx { /// Gets a table database handle if it exists, otherwise creates it. pub fn get_dbi(&self) -> Result { - match self.dbis.entry(T::NAME) { - dashmap::Entry::Occupied(occ) => Ok(*occ.get()), - dashmap::Entry::Vacant(vac) => { - let dbi = self - .inner - .open_db(Some(T::NAME)) - .map(|db| db.dbi()) - .map_err(|e| DatabaseError::Open(e.into()))?; - vac.insert(dbi); - Ok(dbi) - } + if let Some(dbi) = self.dbis.get(T::NAME) { + Ok(*dbi) + } else { + self.inner + .open_db(Some(T::NAME)) + .map(|db| db.dbi()) + .map_err(|e| DatabaseError::Open(e.into())) } } diff --git a/crates/storage/db/src/mdbx.rs b/crates/storage/db/src/mdbx.rs index 9042299afdc..f0cb0332138 100644 --- a/crates/storage/db/src/mdbx.rs +++ b/crates/storage/db/src/mdbx.rs @@ -41,7 +41,7 @@ pub fn init_db_for, TS: TableSet>( args: DatabaseArguments, ) -> eyre::Result { let client_version = args.client_version().clone(); - let db = create_db(path, args)?; + let mut db = create_db(path, args)?; db.create_tables_for::()?; db.record_client_version(client_version)?; Ok(db)