Repository: paritytech/parity-db (language: rust)

Issue:
Memory is "leaking" on Windows and leads to crashes
After many complaints from users of our Substrate-based chain I did some experiments and discovered that with ParityDb application uses a lot of memory on Windows until it runs out of memory, while RocksDb works fine.

I tried both mimalloc allocator and default system allocator with the same result.

Errors typically look something like this (there are other variations, sometimes other components run out of memory first):
```
2024-01-04T15:49:21.505650Z  WARN parity-db: Background worker error: IO Error: The paging file is too small for this operation to complete. (os error 1455)    
```

Interestingly enough, task manager shows very reasonable memory usage of the app in the process list, while there is a huge total memory usage in Performance tab.

In Sysinternals Process Explorer I see that while the app uses ~3G of "Working Set" memory (similarly to what task manager shows), "Private Bytes" is over 10x more (again, not happening with RocksDb).

Not sure how to narrow it down more yet, so open for suggestions and tests, but this is a pretty big issue that I'd like to help resolve.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8635a7ef..763352ff 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,11 @@ The format is based on [Keep a Changelog].
 
 ## [Unreleased]
 
+## [v0.4.13] - 2024-01-05
+
+- Lock memory map when reading from file [`#234`](https://github.com/paritytech/parity-db/pull/234)
+- Disable read-ahead on Windows [`#235`](https://github.com/paritytech/parity-db/pull/235)
+
 ## [v0.4.12] - 2023-10-12
 
 - CI for windows and macos. Also fixes access denied error on windows [`#222`](https://github.com/paritytech/parity-db/pull/222)
diff --git a/Cargo.toml b/Cargo.toml
index 9c38189a..30e7079d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "parity-db"
-version = "0.4.12"
+version = "0.4.13"
 authors = ["Parity Technologies <admin@parity.io>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"
diff --git a/admin/src/bench/mod.rs b/admin/src/bench/mod.rs
index 373242d7..8e91aaa2 100644
--- a/admin/src/bench/mod.rs
+++ b/admin/src/bench/mod.rs
@@ -5,7 +5,7 @@ use super::*;
 
 mod sizes;
 
-pub use parity_db::{CompressionType, Db, Key, Value};
+pub use parity_db::{Db, Key};
 
 use rand::{RngCore, SeedableRng};
 use std::{
diff --git a/src/btree/mod.rs b/src/btree/mod.rs
index e3c456d8..6393a87f 100644
--- a/src/btree/mod.rs
+++ b/src/btree/mod.rs
@@ -16,7 +16,7 @@ use crate::{
 	},
 	Operation,
 };
-pub use iter::{BTreeIterator, LastIndex, LastKey};
+pub use iter::{BTreeIterator, LastKey};
 use node::SeparatorInner;
 
 #[allow(clippy::module_inception)]
diff --git a/src/file.rs b/src/file.rs
index e6e46c06..e07095cf 100644
--- a/src/file.rs
+++ b/src/file.rs
@@ -144,15 +144,22 @@ impl TableFile {
 		Ok(())
 	}
 
-	pub fn slice_at(&self, offset: u64, len: usize) -> &[u8] {
+	#[cfg(not(feature = "loom"))]
+	pub fn slice_at(&self, offset: u64, len: usize) -> MappedBytesGuard {
+		let offset = offset as usize;
+		let map = self.map.read();
+		parking_lot::RwLockReadGuard::map(map, |map| {
+			let (map, _) = map.as_ref().unwrap();
+			&map[offset..offset + len]
+		})
+	}
+
+	#[cfg(feature = "loom")]
+	pub fn slice_at(&self, offset: u64, len: usize) -> MappedBytesGuard {
 		let offset = offset as usize;
 		let map = self.map.read();
 		let (map, _) = map.as_ref().unwrap();
-		let data: &[u8] = unsafe {
-			let ptr = map.as_ptr().add(offset);
-			std::slice::from_raw_parts(ptr, len)
-		};
-		data
+		MappedBytesGuard::new(map[offset..offset + len].to_vec())
 	}
 
 	pub fn write_at(&self, buf: &[u8], offset: u64) -> Result<()> {
@@ -189,8 +196,6 @@ impl TableFile {
 					let new_map = mmap(&file, new_len as usize)?;
 					let old_map = std::mem::replace(map, new_map);
 					try_io!(old_map.flush());
-					// Leak the old mapping as there might be concurrent readers.
-					std::mem::forget(old_map);
 				}
 				new_len
 			},
@@ -217,3 +222,29 @@ impl TableFile {
 		Ok(())
 	}
 }
+
+// Loom is missing support for guard projection, so we copy the data as a workaround.
+#[cfg(feature = "loom")]
+pub struct MappedBytesGuard<'a> {
+	_phantom: std::marker::PhantomData<&'a ()>,
+	data: Vec<u8>,
+}
+
+#[cfg(feature = "loom")]
+impl<'a> MappedBytesGuard<'a> {
+	pub fn new(data: Vec<u8>) -> Self {
+		Self { _phantom: std::marker::PhantomData, data }
+	}
+}
+
+#[cfg(feature = "loom")]
+impl<'a> std::ops::Deref for MappedBytesGuard<'a> {
+	type Target = [u8];
+
+	fn deref(&self) -> &Self::Target {
+		self.data.as_slice()
+	}
+}
+
+#[cfg(not(feature = "loom"))]
+pub type MappedBytesGuard<'a> = parking_lot::MappedRwLockReadGuard<'a, [u8]>;
diff --git a/src/log.rs b/src/log.rs
index 71bf7784..07e59cb2 100644
--- a/src/log.rs
+++ b/src/log.rs
@@ -4,6 +4,7 @@
 use crate::{
 	column::ColId,
 	error::{try_io, Error, Result},
+	file::MappedBytesGuard,
 	index::{Chunk as IndexChunk, TableId as IndexTableId, ENTRY_BYTES},
 	options::Options,
 	parking_lot::{RwLock, RwLockWriteGuard},
@@ -92,32 +93,6 @@ impl LogOverlays {
 	}
 }
 
-// Loom is missing support for guard projection, so we copy the data as a workaround.
-#[cfg(feature = "loom")]
-pub struct MappedBytesGuard<'a> {
-	_phantom: std::marker::PhantomData<&'a ()>,
-	data: Vec<u8>,
-}
-
-#[cfg(feature = "loom")]
-impl<'a> MappedBytesGuard<'a> {
-	fn new(data: Vec<u8>) -> Self {
-		Self { _phantom: std::marker::PhantomData, data }
-	}
-}
-
-#[cfg(feature = "loom")]
-impl<'a> std::ops::Deref for MappedBytesGuard<'a> {
-	type Target = [u8];
-
-	fn deref(&self) -> &Self::Target {
-		self.data.as_slice()
-	}
-}
-
-#[cfg(not(feature = "loom"))]
-type MappedBytesGuard<'a> = parking_lot::MappedRwLockReadGuard<'a, [u8]>;
-
 impl LogQuery for RwLock<LogOverlays> {
 	type ValueRef<'a> = MappedBytesGuard<'a>;
 
diff --git a/src/table.rs b/src/table.rs
index e16d4394..dae909de 100644
--- a/src/table.rs
+++ b/src/table.rs
@@ -382,6 +382,20 @@ impl<B: AsRef<[u8]> + AsMut<[u8]>> std::ops::IndexMut<std::ops::Range<usize>> fo
 	}
 }
 
+enum LockedSlice<O: std::ops::Deref<Target = [u8]>, F: std::ops::Deref<Target = [u8]>> {
+	FromOverlay(O),
+	FromFile(F),
+}
+
+impl<O: std::ops::Deref<Target = [u8]>, F: std::ops::Deref<Target = [u8]>> LockedSlice<O, F> {
+	fn as_slice(&self) -> &[u8] {
+		match self {
+			LockedSlice::FromOverlay(slice) => &*slice,
+			LockedSlice::FromFile(slice) => &*slice,
+		}
+	}
+}
+
 impl ValueTable {
 	pub fn open(
 		path: Arc<std::path::PathBuf>,
@@ -458,14 +472,14 @@ impl ValueTable {
 		let entry_size = self.entry_size as usize;
 		loop {
 			let vbuf = log.value_ref(self.id, index);
-			let buf: &[u8] = if let Some(buf) = vbuf.as_deref() {
+			let buf: LockedSlice<_, _> = if let Some(buf) = vbuf.as_deref() {
 				log::trace!(
 					target: "parity-db",
 					"{}: Found in overlay {}",
 					self.id,
 					index,
 				);
-				buf
+				LockedSlice::FromOverlay(buf)
 			} else {
 				log::trace!(
 					target: "parity-db",
@@ -473,11 +487,10 @@ impl ValueTable {
 					self.id,
 					index,
 				);
-				self.file.slice_at(index * self.entry_size as u64, entry_size)
+				let vbuf = self.file.slice_at(index * self.entry_size as u64, entry_size);
+				LockedSlice::FromFile(vbuf)
 			};
-			let mut buf = EntryRef::new(buf);
-
-			buf.set_offset(0);
+			let mut buf = EntryRef::new(buf.as_slice());
 
 			if buf.is_tombstone() {
 				return Ok((0, false))


Test patch: