Allow for caching of reads (#789)

firewood/src/manager.rs

@@ -14,7 +14,9 @@ use typed_builder::TypedBuilder;
 
 use crate::v2::api::HashKey;
 
-use storage::{Committed, FileBacked, ImmutableProposal, NodeStore, Parentable, TrieHash};
+use storage::{
+    CacheReadStrategy, Committed, FileBacked, ImmutableProposal, NodeStore, Parentable, TrieHash,
+};
 
 #[derive(Clone, Debug, TypedBuilder)]
 /// Revision manager configuratoin
@@ -29,6 +31,9 @@ pub struct RevisionManagerConfig {
 
     #[builder(default_code = "NonZero::new(40000).expect(\"non-zero\")")]
     free_list_cache_size: NonZero<usize>,
+
+    #[builder(default = CacheReadStrategy::WritesOnly)]
+    cache_read_strategy: CacheReadStrategy,
 }
 
 type CommittedRevision = Arc<NodeStore<Committed, FileBacked>>;
@@ -73,6 +78,7 @@ impl RevisionManager {
             config.node_cache_size,
             config.free_list_cache_size,
             truncate,
+            config.cache_read_strategy,
         )?);
         let nodestore = match truncate {
             true => Arc::new(NodeStore::new_empty_committed(storage.clone())?),

storage/src/lib.rs

@@ -34,3 +34,19 @@ pub use linear::filebacked::FileBacked;
 pub use linear::memory::MemStore;
 
 pub use trie_hash::TrieHash;
+
+/// The strategy for caching nodes that are read
+/// from the storage layer. Generally, we only want to
+/// cache write operations, but for some read-heavy workloads
+/// you can enable caching of branch reads or all reads.
+#[derive(Clone, Debug)]
+pub enum CacheReadStrategy {
+    /// Only cache writes (no reads will be cached)
+    WritesOnly,
+
+    /// Cache branch reads (reads that are not leaf nodes)
+    BranchReads,
+
+    /// Cache all reads (leaves and branches)
+    All,
+}

storage/src/linear/filebacked.rs

@@ -16,7 +16,7 @@ use std::sync::{Arc, Mutex};
 use lru::LruCache;
 use metrics::counter;
 
-use crate::{LinearAddress, Node};
+use crate::{CacheReadStrategy, LinearAddress, Node};
 
 use super::{ReadableStorage, WritableStorage};
 
@@ -26,6 +26,7 @@ pub struct FileBacked {
     fd: File,
     cache: Mutex<LruCache<LinearAddress, Arc<Node>>>,
     free_list_cache: Mutex<LruCache<LinearAddress, Option<LinearAddress>>>,
+    cache_read_strategy: CacheReadStrategy,
 }
 
 impl FileBacked {
@@ -35,6 +36,7 @@ impl FileBacked {
         node_cache_size: NonZero<usize>,
         free_list_cache_size: NonZero<usize>,
         truncate: bool,
+        cache_read_strategy: CacheReadStrategy,
     ) -> Result<Self, Error> {
         let fd = OpenOptions::new()
             .read(true)
@@ -47,6 +49,7 @@ impl FileBacked {
             fd,
             cache: Mutex::new(LruCache::new(node_cache_size)),
             free_list_cache: Mutex::new(LruCache::new(free_list_cache_size)),
+            cache_read_strategy,
         })
     }
 }
@@ -74,6 +77,28 @@ impl ReadableStorage for FileBacked {
         counter!("firewood.cache.freelist", "type" => if cached.is_some() { "hit" } else { "miss" }).increment(1);
         cached
     }
+
+    fn cache_read_strategy(&self) -> &CacheReadStrategy {
+        &self.cache_read_strategy
+    }
+
+    fn cache_node(&self, addr: LinearAddress, node: Arc<Node>) {
+        match self.cache_read_strategy {
+            CacheReadStrategy::WritesOnly => {
+                // we don't cache reads
+            }
+            CacheReadStrategy::All => {
+                let mut guard = self.cache.lock().expect("poisoned lock");
+                guard.put(addr, node);
+            }
+            CacheReadStrategy::BranchReads => {
+                if !node.is_leaf() {
+                    let mut guard = self.cache.lock().expect("poisoned lock");
+                    guard.put(addr, node);
+                }
+            }
+        }
+    }
 }
 
 impl WritableStorage for FileBacked {
@@ -169,6 +194,7 @@ mod test {
             NonZero::new(10).unwrap(),
             NonZero::new(10).unwrap(),
             false,
+            CacheReadStrategy::WritesOnly,
         )
         .unwrap();
         let mut reader = fb.stream_from(0).unwrap();
@@ -208,6 +234,7 @@ mod test {
             NonZero::new(10).unwrap(),
             NonZero::new(10).unwrap(),
             false,
+            CacheReadStrategy::WritesOnly,
         )
         .unwrap();
         let mut reader = fb.stream_from(0).unwrap();

storage/src/linear/mod.rs

@@ -22,7 +22,7 @@ use std::io::{Error, Read};
 use std::num::NonZero;
 use std::sync::Arc;
 
-use crate::{LinearAddress, Node};
+use crate::{CacheReadStrategy, LinearAddress, Node};
 pub(super) mod filebacked;
 pub mod memory;
 
@@ -51,6 +51,14 @@ pub trait ReadableStorage: Debug + Sync + Send {
     fn free_list_cache(&self, _addr: LinearAddress) -> Option<Option<LinearAddress>> {
         None
     }
+
+    /// Return the cache read strategy for this readable storage
+    fn cache_read_strategy(&self) -> &CacheReadStrategy {
+        &CacheReadStrategy::WritesOnly
+    }
+
+    /// Cache a node for future reads
+    fn cache_node(&self, _addr: LinearAddress, _node: Arc<Node>) {}
 }
 
 /// Trait for writable storage.

storage/src/nodestore.rs

@@ -61,7 +61,7 @@ use std::sync::Arc;
 
 use crate::hashednode::hash_node;
 use crate::node::{ByteCounter, Node};
-use crate::{Child, FileBacked, Path, ReadableStorage, TrieHash};
+use crate::{CacheReadStrategy, Child, FileBacked, Path, ReadableStorage, TrieHash};
 
 use super::linear::WritableStorage;
 
@@ -217,13 +217,24 @@ impl<T: ReadInMemoryNode, S: ReadableStorage> NodeStore<T, S> {
 
         debug_assert!(addr.get() % 8 == 0);
 
-        let addr = addr.get() + 1; // skip the length byte
+        let actual_addr = addr.get() + 1; // skip the length byte
 
         let _span = LocalSpan::enter_with_local_parent("read_and_deserialize");
 
-        let area_stream = self.storage.stream_from(addr)?;
-        let node = Node::from_reader(area_stream)?;
-        Ok(node.into())
+        let area_stream = self.storage.stream_from(actual_addr)?;
+        let node = Arc::new(Node::from_reader(area_stream)?);
+        match self.storage.cache_read_strategy() {
+            CacheReadStrategy::All => {
+                self.storage.cache_node(addr, node.clone());
+            }
+            CacheReadStrategy::BranchReads => {
+                if !node.is_leaf() {
+                    self.storage.cache_node(addr, node.clone());
+                }
+            }
+            CacheReadStrategy::WritesOnly => {}
+        }
+        Ok(node)
     }
 }