Implement read cache for bytecode

voltrevo · Dec 21, 2023 · b21da6f · b21da6f
1 parent 63798c4
commit b21da6f
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 3 deletions.
diff --git a/valuescript_vm/src/val_storage.rs b/valuescript_vm/src/val_storage.rs
@@ -399,11 +399,29 @@ fn read_ref_bytecode_from_entry<SB: StorageBackend, Tx: StorageReader<SB>>(
   tx: &Tx,
   reader: &mut StorageEntryReader,
 ) -> Result<Rc<Bytecode>, GenericError> {
+  let ref_cell = tx
+    .get_backend()
+    .upgrade()
+    .ok_or(Into::<Box<dyn Error>>::into("Storage backend missing"))?;
+
   let ptr = reader.read_ref()?;
+
+  let borrow = ref_cell.borrow();
+  let mut read_cache = borrow.get_read_cache();
+
+  if let Some(cache_box) = read_cache.get(&ptr.data) {
+    if let Some(cache_val) = cache_box.downcast_ref::<Rc<Bytecode>>() {
+      return Ok(cache_val.clone());
+    }
+  }
+
   let entry = tx.read_or_err(ptr)?;
 
-  // TODO: Cached reads
-  Ok(Rc::new(Bytecode::from_storage_entry(tx, entry)?))
+  let res = Rc::new(Bytecode::from_storage_entry(tx, entry)?);
+
+  read_cache.insert(ptr.data, Box::new(res.clone()));
+
+  Ok(res)
 }
 
 fn write_ref_bytecode_to_entry<SB: StorageBackend, Tx: StorageTxMut<SB>>(

diff --git a/valuescript_vm/src/vs_storage_ptr.rs b/valuescript_vm/src/vs_storage_ptr.rs
@@ -25,17 +25,21 @@ impl<SB: StorageBackend> ValResolver for StorageValAutoPtr<SB> {
     };
 
     let borrow = sb.borrow();
-    let mut read_cache = borrow.get_read_cache();
+    let read_cache = borrow.get_read_cache();
 
     if let Some(cache_box) = read_cache.get(&self.ptr.ptr.data) {
       if let Some(cache_val) = cache_box.downcast_ref::<Val>() {
         return cache_val.clone();
       }
     }
 
+    drop(read_cache);
+
     let entry = sb.read(self.ptr.ptr).unwrap().expect("Unresolved ptr");
     let res = Val::from_storage_entry(&sb, entry).expect("Failed to deserialize Val");
 
+    let mut read_cache = borrow.get_read_cache();
+
     read_cache.insert(self.ptr.ptr.data, Box::new(res.clone()));
 
     res
@@ -64,6 +68,9 @@ impl VsStoragePtr {
   pub fn new<SB: StorageBackend + 'static>(auto_ptr: StorageAutoPtr<SB, Val>) -> Self {
     Self {
       resolver: Box::new(StorageValAutoPtr { ptr: auto_ptr }),
+
+      // TODO: Since there's also caching in the storage backend's read_cache, is it worthwhile to
+      // cache here too? (Perf test.)
       cache: RefCell::new(None),
     }
   }