From a4e63346f237cb4f517d5a544e8b669bda87ff63 Mon Sep 17 00:00:00 2001 From: Christopher Berner Date: Sat, 3 Feb 2024 13:35:41 -0800 Subject: [PATCH] Move len(), is_empty(), and stats() to separate trait --- benches/multithreaded_insert_benchmark.rs | 2 +- fuzz/fuzz_targets/fuzz_redb.rs | 2 +- src/lib.rs | 4 +- src/multimap_table.rs | 146 ++++++++++------------ src/table.rs | 100 +++++++-------- tests/backward_compatibility.rs | 2 +- tests/basic_tests.rs | 2 +- tests/integration_tests.rs | 3 +- tests/multimap_tests.rs | 4 +- tests/multithreading_tests.rs | 2 +- 10 files changed, 129 insertions(+), 138 deletions(-) diff --git a/benches/multithreaded_insert_benchmark.rs b/benches/multithreaded_insert_benchmark.rs index eb6651e6..2ad1e5ec 100644 --- a/benches/multithreaded_insert_benchmark.rs +++ b/benches/multithreaded_insert_benchmark.rs @@ -4,7 +4,7 @@ use tempfile::NamedTempFile; use rand::rngs::StdRng; use rand::{Rng, SeedableRng}; -use redb::{Database, ReadableTable, TableDefinition}; +use redb::{Database, ReadableTableMetadata, TableDefinition}; use std::time::Instant; const ELEMENTS: u64 = 1_000_000; diff --git a/fuzz/fuzz_targets/fuzz_redb.rs b/fuzz/fuzz_targets/fuzz_redb.rs index e9988007..a7809486 100644 --- a/fuzz/fuzz_targets/fuzz_redb.rs +++ b/fuzz/fuzz_targets/fuzz_redb.rs @@ -1,7 +1,7 @@ #![no_main] use libfuzzer_sys::fuzz_target; -use redb::{AccessGuard, Database, Durability, Error, MultimapTable, MultimapTableDefinition, MultimapValue, ReadableMultimapTable, ReadableTable, Savepoint, StorageBackend, Table, TableDefinition, WriteTransaction}; +use redb::{AccessGuard, Database, Durability, Error, MultimapTable, MultimapTableDefinition, MultimapValue, ReadableMultimapTable, ReadableTable, ReadableTableMetadata, Savepoint, StorageBackend, Table, TableDefinition, WriteTransaction}; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::fmt::Debug; use std::io::{ErrorKind, Read, Seek, SeekFrom}; diff --git a/src/lib.rs b/src/lib.rs index 65543649..d62a2bb9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -66,8 +66,8 @@ pub use multimap_table::{ ReadOnlyUntypedMultimapTable, ReadableMultimapTable, }; pub use table::{ - Drain, DrainFilter, Range, ReadOnlyTable, ReadOnlyUntypedTable, ReadableTable, Table, - TableStats, + Drain, DrainFilter, Range, ReadOnlyTable, ReadOnlyUntypedTable, ReadableTable, + ReadableTableMetadata, Table, TableStats, }; pub use transactions::{DatabaseStats, Durability, ReadTransaction, WriteTransaction}; pub use tree_store::{AccessGuard, AccessGuardMut, Savepoint}; diff --git a/src/multimap_table.rs b/src/multimap_table.rs index cbb824d5..bcd31e53 100644 --- a/src/multimap_table.rs +++ b/src/multimap_table.rs @@ -1,7 +1,7 @@ use crate::db::TransactionGuard; use crate::multimap_table::DynamicCollectionType::{Inline, Subtree}; use crate::sealed::Sealed; -use crate::table::TableStats; +use crate::table::{ReadableTableMetadata, TableStats}; use crate::tree_store::{ btree_stats, AllPageNumbersBtreeIter, BranchAccessor, Btree, BtreeMut, BtreeRangeIter, BtreeStats, CachePriority, Checksum, LeafAccessor, LeafMutator, Page, PageHint, PageNumber, @@ -1099,6 +1099,39 @@ impl<'txn, K: Key + 'static, V: Key + 'static> MultimapTable<'txn, K, V> { } } +impl<'txn, K: Key + 'static, V: Key + 'static> ReadableTableMetadata for MultimapTable<'txn, K, V> { + fn stats(&self) -> Result { + let tree_stats = multimap_btree_stats( + self.tree.get_root().map(|(p, _)| p), + &self.mem, + K::fixed_width(), + V::fixed_width(), + )?; + + Ok(TableStats { + tree_height: tree_stats.tree_height, + leaf_pages: tree_stats.leaf_pages, + branch_pages: tree_stats.branch_pages, + stored_leaf_bytes: tree_stats.stored_leaf_bytes, + metadata_bytes: tree_stats.metadata_bytes, + fragmented_bytes: tree_stats.fragmented_bytes, + }) + } + + /// Returns the number of key-value pairs in the table + fn len(&self) -> Result { + let mut count = 0; + for item in self.iter()? { + let (_, values) = item?; + for v in values { + v?; + count += 1; + } + } + Ok(count) + } +} + impl<'txn, K: Key + 'static, V: Key + 'static> ReadableMultimapTable for MultimapTable<'txn, K, V> { @@ -1133,42 +1166,6 @@ impl<'txn, K: Key + 'static, V: Key + 'static> ReadableMultimapTable self.mem.clone(), )) } - - fn stats(&self) -> Result { - let tree_stats = multimap_btree_stats( - self.tree.get_root().map(|(p, _)| p), - &self.mem, - K::fixed_width(), - V::fixed_width(), - )?; - - Ok(TableStats { - tree_height: tree_stats.tree_height, - leaf_pages: tree_stats.leaf_pages, - branch_pages: tree_stats.branch_pages, - stored_leaf_bytes: tree_stats.stored_leaf_bytes, - metadata_bytes: tree_stats.metadata_bytes, - fragmented_bytes: tree_stats.fragmented_bytes, - }) - } - - /// Returns the number of key-value pairs in the table - fn len(&self) -> Result { - let mut count = 0; - for item in self.iter()? { - let (_, values) = item?; - for v in values { - v?; - count += 1; - } - } - Ok(count) - } - - /// Returns `true` if the table is empty - fn is_empty(&self) -> Result { - self.len().map(|x| x == 0) - } } impl Sealed for MultimapTable<'_, K, V> {} @@ -1179,7 +1176,7 @@ impl<'txn, K: Key + 'static, V: Key + 'static> Drop for MultimapTable<'txn, K, V } } -pub trait ReadableMultimapTable: Sealed { +pub trait ReadableMultimapTable: ReadableTableMetadata { /// Returns an iterator over all values for the given key. Values are in ascending order. fn get<'a>(&self, key: impl Borrow>) -> Result> where @@ -1190,13 +1187,6 @@ pub trait ReadableMultimapTable: Sealed { K: 'a, KR: Borrow> + 'a; - /// Retrieves information about storage usage for the table - fn stats(&self) -> Result; - - fn len(&self) -> Result; - - fn is_empty(&self) -> Result; - /// Returns an double-ended iterator over all elements in the table. Values are in ascending /// order. fn iter(&self) -> Result> { @@ -1305,6 +1295,38 @@ impl ReadOnlyMultimapTable { } } +impl ReadableTableMetadata for ReadOnlyMultimapTable { + fn stats(&self) -> Result { + let tree_stats = multimap_btree_stats( + self.tree.get_root().map(|(p, _)| p), + &self.mem, + K::fixed_width(), + V::fixed_width(), + )?; + + Ok(TableStats { + tree_height: tree_stats.tree_height, + leaf_pages: tree_stats.leaf_pages, + branch_pages: tree_stats.branch_pages, + stored_leaf_bytes: tree_stats.stored_leaf_bytes, + metadata_bytes: tree_stats.metadata_bytes, + fragmented_bytes: tree_stats.fragmented_bytes, + }) + } + + fn len(&self) -> Result { + let mut count = 0; + for item in self.iter()? { + let (_, values) = item?; + for v in values { + v?; + count += 1; + } + } + Ok(count) + } +} + impl ReadableMultimapTable for ReadOnlyMultimapTable { @@ -1337,40 +1359,6 @@ impl ReadableMultimapTable self.mem.clone(), )) } - - fn stats(&self) -> Result { - let tree_stats = multimap_btree_stats( - self.tree.get_root().map(|(p, _)| p), - &self.mem, - K::fixed_width(), - V::fixed_width(), - )?; - - Ok(TableStats { - tree_height: tree_stats.tree_height, - leaf_pages: tree_stats.leaf_pages, - branch_pages: tree_stats.branch_pages, - stored_leaf_bytes: tree_stats.stored_leaf_bytes, - metadata_bytes: tree_stats.metadata_bytes, - fragmented_bytes: tree_stats.fragmented_bytes, - }) - } - - fn len(&self) -> Result { - let mut count = 0; - for item in self.iter()? { - let (_, values) = item?; - for v in values { - v?; - count += 1; - } - } - Ok(count) - } - - fn is_empty(&self) -> Result { - self.len().map(|x| x == 0) - } } impl Sealed for ReadOnlyMultimapTable {} diff --git a/src/table.rs b/src/table.rs index 0d243d17..59eca872 100644 --- a/src/table.rs +++ b/src/table.rs @@ -215,24 +215,7 @@ impl<'txn, K: Key + 'static, V: MutInPlaceValue + 'static> Table<'txn, K, V> { } } -impl<'txn, K: Key + 'static, V: Value + 'static> ReadableTable for Table<'txn, K, V> { - fn get<'a>(&self, key: impl Borrow>) -> Result>> - where - K: 'a, - { - self.tree.get(key.borrow()) - } - - fn range<'a, KR>(&self, range: impl RangeBounds + 'a) -> Result> - where - K: 'a, - KR: Borrow> + 'a, - { - self.tree - .range(&range) - .map(|x| Range::new(x, self.transaction.transaction_guard())) - } - +impl<'txn, K: Key + 'static, V: Value + 'static> ReadableTableMetadata for Table<'txn, K, V> { fn stats(&self) -> Result { let tree_stats = self.tree.stats()?; @@ -249,9 +232,24 @@ impl<'txn, K: Key + 'static, V: Value + 'static> ReadableTable for Table<' fn len(&self) -> Result { self.tree.len() } +} - fn is_empty(&self) -> Result { - self.len().map(|x| x == 0) +impl<'txn, K: Key + 'static, V: Value + 'static> ReadableTable for Table<'txn, K, V> { + fn get<'a>(&self, key: impl Borrow>) -> Result>> + where + K: 'a, + { + self.tree.get(key.borrow()) + } + + fn range<'a, KR>(&self, range: impl RangeBounds + 'a) -> Result> + where + K: 'a, + KR: Borrow> + 'a, + { + self.tree + .range(&range) + .map(|x| Range::new(x, self.transaction.transaction_guard())) } } @@ -312,7 +310,20 @@ impl Debug for Table<'_, K, V> { } } -pub trait ReadableTable: Sealed { +pub trait ReadableTableMetadata: Sealed { + /// Retrieves information about storage usage for the table + fn stats(&self) -> Result; + + /// Returns the number of entries in the table + fn len(&self) -> Result; + + /// Returns `true` if the table is empty + fn is_empty(&self) -> Result { + Ok(self.len()? == 0) + } +} + +pub trait ReadableTable: ReadableTableMetadata { /// Returns the value corresponding to the given key fn get<'a>(&self, key: impl Borrow>) -> Result>> where @@ -365,15 +376,6 @@ pub trait ReadableTable: Sealed { self.iter()?.next_back().transpose() } - /// Retrieves information about storage usage for the table - fn stats(&self) -> Result; - - /// Returns the number of entries in the table - fn len(&self) -> Result; - - /// Returns `true` if the table is empty - fn is_empty(&self) -> Result; - /// Returns a double-ended iterator over all elements in the table fn iter(&self) -> Result> { self.range::>(..) @@ -446,24 +448,7 @@ impl ReadOnlyTable { } } -impl ReadableTable for ReadOnlyTable { - fn get<'a>(&self, key: impl Borrow>) -> Result>> - where - K: 'a, - { - self.tree.get(key.borrow()) - } - - fn range<'a, KR>(&self, range: impl RangeBounds + 'a) -> Result> - where - K: 'a, - KR: Borrow> + 'a, - { - self.tree - .range(&range) - .map(|x| Range::new(x, self.transaction_guard.clone())) - } - +impl ReadableTableMetadata for ReadOnlyTable { fn stats(&self) -> Result { let tree_stats = self.tree.stats()?; @@ -480,9 +465,24 @@ impl ReadableTable for ReadOnlyTable fn len(&self) -> Result { self.tree.len() } +} - fn is_empty(&self) -> Result { - self.len().map(|x| x == 0) +impl ReadableTable for ReadOnlyTable { + fn get<'a>(&self, key: impl Borrow>) -> Result>> + where + K: 'a, + { + self.tree.get(key.borrow()) + } + + fn range<'a, KR>(&self, range: impl RangeBounds + 'a) -> Result> + where + K: 'a, + KR: Borrow> + 'a, + { + self.tree + .range(&range) + .map(|x| Range::new(x, self.transaction_guard.clone())) } } diff --git a/tests/backward_compatibility.rs b/tests/backward_compatibility.rs index cdb392c1..2cc035db 100644 --- a/tests/backward_compatibility.rs +++ b/tests/backward_compatibility.rs @@ -1,4 +1,4 @@ -use redb::ReadableTable; +use redb::{ReadableTable, ReadableTableMetadata}; const ELEMENTS: usize = 3; diff --git a/tests/basic_tests.rs b/tests/basic_tests.rs index 97532c6f..299162df 100644 --- a/tests/basic_tests.rs +++ b/tests/basic_tests.rs @@ -1,7 +1,7 @@ use redb::backends::InMemoryBackend; use redb::{ Database, Key, MultimapTableDefinition, MultimapTableHandle, Range, ReadableTable, - TableDefinition, TableError, TableHandle, TypeName, Value, + ReadableTableMetadata, TableDefinition, TableError, TableHandle, TypeName, Value, }; use std::cmp::Ordering; #[cfg(not(target_os = "wasi"))] diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 59a61bdf..cfc99537 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -4,7 +4,8 @@ use std::io::ErrorKind; use rand::prelude::SliceRandom; use rand::Rng; use redb::{ - Builder, Database, Durability, MultimapTableDefinition, ReadableTable, TableDefinition, + Builder, Database, Durability, MultimapTableDefinition, ReadableTable, ReadableTableMetadata, + TableDefinition, }; use redb::{DatabaseError, ReadableMultimapTable, SavepointError, StorageError, TableError}; diff --git a/tests/multimap_tests.rs b/tests/multimap_tests.rs index 45ac8586..f29592da 100644 --- a/tests/multimap_tests.rs +++ b/tests/multimap_tests.rs @@ -1,4 +1,6 @@ -use redb::{Database, MultimapTableDefinition, ReadableMultimapTable, TableError}; +use redb::{ + Database, MultimapTableDefinition, ReadableMultimapTable, ReadableTableMetadata, TableError, +}; const STR_TABLE: MultimapTableDefinition<&str, &str> = MultimapTableDefinition::new("str_to_str"); const SLICE_U64_TABLE: MultimapTableDefinition<&[u8], u64> = diff --git a/tests/multithreading_tests.rs b/tests/multithreading_tests.rs index 63c428bc..e87ec628 100644 --- a/tests/multithreading_tests.rs +++ b/tests/multithreading_tests.rs @@ -1,6 +1,6 @@ #[cfg(not(target_os = "wasi"))] mod multithreading_test { - use redb::{Database, ReadableTable, TableDefinition}; + use redb::{Database, ReadableTableMetadata, TableDefinition}; use std::sync::Arc; use std::thread;