Skip to content

Commit

Permalink
Avoid unnecessary tranasctions, fix ptr indirection
Browse files Browse the repository at this point in the history
  • Loading branch information
voltrevo committed Nov 2, 2023
1 parent 0c4ae6f commit 9dee550
Show file tree
Hide file tree
Showing 15 changed files with 65 additions and 88 deletions.
23 changes: 6 additions & 17 deletions storage/src/demo_val.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use std::error::Error;
use std::rc::Rc;

use crate::rc_key::RcKey;
use crate::storage_entity::StorageEntity;
use crate::storage_entry::{StorageEntry, StorageEntryReader};
use crate::storage_io::{StorageReader, StorageTxMut};
use crate::storage_ptr::StorageEntryPtr;
use crate::{GenericError, Storage, StorageBackend};
use crate::{GenericError, StorageBackend};

const NUMBER_TAG: u8 = 0;
const ARRAY_TAG: u8 = 1;
Expand All @@ -20,16 +19,6 @@ pub enum DemoVal {
}

impl DemoVal {
pub(crate) fn numbers<SB: StorageBackend>(
&self,
storage: &mut Storage<SB>,
) -> Result<Vec<u64>, Box<dyn Error>> {
storage
.sb
.borrow()
.transaction(Rc::downgrade(&storage.sb), |sb| self.numbers_impl(sb))
}

fn write_to_entry<SB: StorageBackend, Tx: StorageTxMut<SB>>(
&self,
tx: &mut Tx,
Expand Down Expand Up @@ -63,7 +52,7 @@ impl DemoVal {
}

fn read_from_entry<SB: StorageBackend, Tx: StorageReader<SB>>(
_tx: &mut Tx,
_tx: &Tx,
reader: &mut StorageEntryReader,
) -> Result<DemoVal, GenericError> {
let tag = reader.read_u8()?;
Expand Down Expand Up @@ -91,21 +80,21 @@ impl DemoVal {
})
}

fn numbers_impl<SB: StorageBackend, Tx: StorageReader<SB>>(
pub fn numbers<SB: StorageBackend, Tx: StorageReader<SB>>(
&self,
tx: &mut Tx,
) -> Result<Vec<u64>, GenericError> {
match &self {
DemoVal::Number(n) => Ok(vec![*n]),
DemoVal::Ptr(ptr) => {
let entry = tx.read_or_err(*ptr)?;
Self::from_storage_entry(tx, entry)?.numbers_impl(tx)
Self::from_storage_entry(tx, entry)?.numbers(tx)
}
DemoVal::Array(arr) => {
let mut numbers = Vec::new();

for item in arr.iter() {
numbers.extend(item.numbers_impl(tx)?);
numbers.extend(item.numbers(tx)?);
}

Ok(numbers)
Expand Down Expand Up @@ -146,7 +135,7 @@ impl<SB: StorageBackend> StorageEntity<SB> for DemoVal {
}

fn from_storage_entry<'a, Tx: StorageReader<SB>>(
tx: &mut Tx,
tx: &Tx,
entry: StorageEntry,
) -> Result<Self, GenericError> {
Self::read_from_entry(tx, &mut StorageEntryReader::new(&entry))
Expand Down
39 changes: 4 additions & 35 deletions storage/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ use std::cell::RefCell;
use std::error::Error;
use std::rc::Rc;

use crate::errors::error_str;
use crate::storage_auto_ptr::StorageAutoPtr;
use crate::storage_entity::StorageEntity;
use crate::storage_ptr::{tmp_at_ptr, tmp_count_ptr, StorageEntryPtr, StorageHeadPtr};
use crate::{StorageBackend, StorageReader, StorageTxMut};
Expand All @@ -19,36 +17,6 @@ impl<SB: StorageBackend> Storage<SB> {
}
}

pub fn get_head<SE: StorageEntity<SB>>(
&mut self,
ptr: StorageHeadPtr,
) -> Result<Option<SE>, Box<dyn Error>> {
self
.sb
.borrow()
.transaction(Rc::downgrade(&self.sb), |sb| sb.get_head(ptr))
}

pub fn get<SE: StorageEntity<SB>>(&self, ptr: StorageEntryPtr) -> Result<SE, Box<dyn Error>> {
// TODO: Avoid going through a transaction when read-only
self.sb.borrow().transaction(Rc::downgrade(&self.sb), |sb| {
let entry = sb.read(ptr)?.ok_or(error_str("Ptr not found"))?;

SE::from_storage_entry(sb, entry)
})
}

pub fn get_auto_ptr<SE: StorageEntity<SB>>(
&mut self,
ptr: StorageEntryPtr,
) -> StorageAutoPtr<SB, SE> {
StorageAutoPtr {
_marker: std::marker::PhantomData,
sb: Rc::downgrade(&self.sb),
ptr,
}
}

pub fn set_head<SE: StorageEntity<SB>>(
&mut self,
ptr: StorageHeadPtr,
Expand Down Expand Up @@ -113,9 +81,10 @@ impl<SB: StorageBackend> Storage<SB> {
&mut self,
ptr: StorageEntryPtr,
) -> Result<Option<u64>, Box<dyn Error>> {
self.sb.borrow().transaction(Rc::downgrade(&self.sb), |sb| {
Ok(sb.read(ptr)?.map(|entry| entry.ref_count))
})
self
.sb
.read(ptr)
.map(|entry| entry.map(|entry| entry.ref_count))
}
}

Expand Down
12 changes: 5 additions & 7 deletions storage/src/storage_auto_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ impl<SB: StorageBackend, SE: StorageEntity<SB>> StorageAutoPtr<SB, SE> {
None => return Err("Storage backend dropped".into()),
};

let res = sb.borrow().transaction(self.sb.clone(), |sb| {
Ok(match sb.read(self.ptr)? {
Some(entry) => Some(SE::from_storage_entry(sb, entry)?),
None => None,
})
});
let res = match sb.read(self.ptr)? {
Some(entry) => Some(SE::from_storage_entry(&sb, entry)?),
None => None,
};

res
Ok(res)
}
}
16 changes: 15 additions & 1 deletion storage/src/storage_backend.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use std::{cell::RefCell, error::Error, rc::Weak};
use std::{
cell::RefCell,
error::Error,
rc::{Rc, Weak},
};

use crate::{
storage_io::{StorageReader, StorageTxMut},
Expand Down Expand Up @@ -29,3 +33,13 @@ pub trait StorageBackend: Sized {
#[cfg(test)]
fn len(&self) -> usize;
}

impl<SB: StorageBackend> StorageReader<SB> for Rc<RefCell<SB>> {
fn read_bytes<T>(&self, ptr: StoragePtr<T>) -> Result<Option<Vec<u8>>, GenericError> {
self.borrow().read_bytes(ptr)
}

fn get_backend(&self) -> Weak<RefCell<SB>> {
Rc::downgrade(self)
}
}
2 changes: 1 addition & 1 deletion storage/src/storage_entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub trait StorageEntity<SB: StorageBackend>: Sized {
) -> Result<StorageEntry, GenericError>;

fn from_storage_entry<Tx: StorageReader<SB>>(
tx: &mut Tx,
tx: &Tx,
entry: StorageEntry,
) -> Result<Self, GenericError>;
}
21 changes: 12 additions & 9 deletions storage/src/storage_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,15 @@ use rand::thread_rng;
use serde::{Deserialize, Serialize};

use crate::{
GenericError, RcKey, StorageAutoPtr, StorageBackend, StorageEntity, StorageEntryPtr,
StorageHeadPtr, StoragePtr,
errors::error_str, GenericError, RcKey, StorageAutoPtr, StorageBackend, StorageEntity,
StorageEntryPtr, StorageHeadPtr, StoragePtr,
};

pub trait StorageReader<SB: StorageBackend>: Sized {
fn read_bytes<T>(&self, ptr: StoragePtr<T>) -> Result<Option<Vec<u8>>, GenericError>;
fn get_backend(&self) -> Weak<RefCell<SB>>;

fn get_auto_ptr<SE: StorageEntity<SB>>(
&mut self,
ptr: StorageEntryPtr,
) -> StorageAutoPtr<SB, SE> {
fn get_auto_ptr<SE: StorageEntity<SB>>(&self, ptr: StorageEntryPtr) -> StorageAutoPtr<SB, SE> {
StorageAutoPtr {
_marker: std::marker::PhantomData,
sb: self.get_backend(),
Expand All @@ -24,7 +21,7 @@ pub trait StorageReader<SB: StorageBackend>: Sized {
}

fn read<T: for<'de> Deserialize<'de>>(
&mut self,
&self,
ptr: StoragePtr<T>,
) -> Result<Option<T>, GenericError> {
let data = match self.read_bytes(ptr)? {
Expand All @@ -36,7 +33,7 @@ pub trait StorageReader<SB: StorageBackend>: Sized {
}

fn read_or_err<T: for<'de> Deserialize<'de>>(
&mut self,
&self,
ptr: StoragePtr<T>,
) -> Result<T, GenericError> {
match self.read(ptr)? {
Expand All @@ -46,7 +43,7 @@ pub trait StorageReader<SB: StorageBackend>: Sized {
}

fn get_head<SE: StorageEntity<SB>>(
&mut self,
&self,
head_ptr: StorageHeadPtr,
) -> Result<Option<SE>, GenericError> {
let entry_ptr = match self.read(head_ptr)? {
Expand All @@ -61,6 +58,12 @@ pub trait StorageReader<SB: StorageBackend>: Sized {

SE::from_storage_entry(self, entry).map(Some)
}

fn get<SE: StorageEntity<SB>>(&self, ptr: StorageEntryPtr) -> Result<SE, GenericError> {
let entry = self.read(ptr)?.ok_or(error_str("Ptr not found"))?;

SE::from_storage_entry(self, entry)
}
}

pub trait StorageTxMut<SB: StorageBackend>: StorageReader<SB> + Sized {
Expand Down
2 changes: 1 addition & 1 deletion storage/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ mod tests_ {

use crate::{
demo_val::DemoVal, memory_backend::MemoryBackend, sled_backend::SledBackend, storage::Storage,
storage_ptr::storage_head_ptr, StorageBackend,
storage_ptr::storage_head_ptr, StorageBackend, StorageReader,
};

fn run(impl_memory: fn(&mut Storage<MemoryBackend>), impl_sled: fn(&mut Storage<SledBackend>)) {
Expand Down
2 changes: 1 addition & 1 deletion valuescript_vm/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl<SB: StorageBackend> StorageEntity<SB> for Bytecode {
}

fn from_storage_entry<Tx: StorageReader<SB>>(
_tx: &mut Tx,
_tx: &Tx,
entry: storage::StorageEntry,
) -> Result<Self, GenericError> {
Ok(Bytecode::new(entry.data))
Expand Down
11 changes: 9 additions & 2 deletions valuescript_vm/src/native_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,22 @@ impl<'a> ThisWrapper<'a> {
ThisWrapper { const_, this }
}

pub fn get(&self) -> &Val {
self.this
pub fn get(&self) -> Val {
match &self.this {
Val::StoragePtr(ptr) => ptr.get(),
_ => self.this.clone(),
}
}

pub fn get_mut(&mut self) -> Result<&mut Val, Val> {
if self.const_ {
return Err("Cannot mutate this because it is const".to_type_error());
}

if let Val::StoragePtr(ptr) = &self.this {
*self.this = ptr.get()
}

Ok(self.this)
}
}
Expand Down
6 changes: 3 additions & 3 deletions valuescript_vm/src/number_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ static TO_EXPONENTIAL: NativeFunction = native_fn(|this, params| {
return Err("precision must be between 0 and 100".to_range_error());
}

format_exponential(*number, Some(precision as usize))
format_exponential(number, Some(precision as usize))
}
None => format_exponential(*number, None),
None => format_exponential(number, None),
},
_ => return Err("number indirection".to_internal_error()),
})
Expand All @@ -91,7 +91,7 @@ static TO_STRING: NativeFunction = native_fn(|this, params| {

static VALUE_OF: NativeFunction = native_fn(|this, _params| {
Ok(match this.get() {
Val::Number(number) => Val::Number(*number),
Val::Number(number) => Val::Number(number),
_ => return Err("number indirection".to_internal_error()),
})
});
Expand Down
2 changes: 1 addition & 1 deletion valuescript_vm/src/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ static BOOL_TO_STRING: NativeFunction = native_fn(|this, _params| {

static BOOL_VALUE_OF: NativeFunction = native_fn(|this, _params| {
Ok(match this.get() {
Val::Bool(b) => Val::Bool(*b),
Val::Bool(b) => Val::Bool(b),
_ => return Err("bool indirection".to_type_error()),
})
});
4 changes: 2 additions & 2 deletions valuescript_vm/src/string_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ static PAD_START: NativeFunction = native_fn(|this, params| {
}
}

prefix.push_str(string_data);
prefix.push_str(&string_data);

prefix.to_val()
}
Expand All @@ -426,7 +426,7 @@ static REPEAT: NativeFunction = native_fn(|this, params| {
let mut result = String::new();

for _ in 0..count {
result.push_str(string_data);
result.push_str(&string_data);
}

result.to_val()
Expand Down
10 changes: 3 additions & 7 deletions valuescript_vm/src/val_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,11 @@ impl Tag {

impl<SB: StorageBackend + 'static> StorageEntity<SB> for Val {
fn from_storage_entry<Tx: StorageReader<SB>>(
tx: &mut Tx,
tx: &Tx,
entry: StorageEntry,
) -> Result<Self, GenericError> {
let mut reader = StorageEntryReader::new(&entry);
let res = read_from_entry(tx, &mut reader);
match &res {
Ok(val) => println!("val: {}", val.pretty()),
Err(_) => println!("error"),
}
assert!(reader.done());

res
Expand Down Expand Up @@ -267,7 +263,7 @@ fn write_ptr_to_entry<SB: StorageBackend + 'static, Tx: StorageTxMut<SB>>(
}

fn read_from_entry<SB: StorageBackend + 'static, Tx: StorageReader<SB>>(
tx: &mut Tx,
tx: &Tx,
reader: &mut StorageEntryReader,
) -> Result<Val, GenericError> {
let tag = Tag::from_byte(reader.read_u8()?)?;
Expand Down Expand Up @@ -400,7 +396,7 @@ fn read_symbol_from_entry(reader: &mut StorageEntryReader) -> Result<VsSymbol, B
}

fn read_ref_bytecode_from_entry<SB: StorageBackend, Tx: StorageReader<SB>>(
tx: &mut Tx,
tx: &Tx,
reader: &mut StorageEntryReader,
) -> Result<Rc<Bytecode>, GenericError> {
let ptr = reader.read_ref()?;
Expand Down
1 change: 1 addition & 0 deletions valuescript_vm/src/vs_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ impl ValTrait for Val {
Function(f) => LoadFunctionResult::StackFrame(f.make_frame()),
Static(s) => s.load_function(),
Dynamic(val) => val.load_function(),
StoragePtr(ptr) => ptr.get().load_function(),

_ => LoadFunctionResult::NotAFunction,
};
Expand Down
2 changes: 1 addition & 1 deletion vstc/src/db_command.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{process::exit, rc::Rc};

use storage::{storage_head_ptr, SledBackend, Storage};
use storage::{storage_head_ptr, SledBackend, Storage, StorageReader};
use valuescript_compiler::asm;
use valuescript_vm::{
vs_value::{ToVal, Val},
Expand Down

0 comments on commit 9dee550

Please sign in to comment.