From 5e708b972085c615a2ee84c34d2748a2b10cdc2f Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 21 Jan 2017 18:38:09 +0100 Subject: [PATCH] Change FileLockEntryHandle to operate on cached store objects --- libimagruby/Cargo.toml | 1 + libimagruby/src/cache.rs | 40 +++--------- libimagruby/src/entry.rs | 131 +++++++++++++++++++++++++++++++-------- libimagruby/src/lib.rs | 1 + 4 files changed, 113 insertions(+), 60 deletions(-) diff --git a/libimagruby/Cargo.toml b/libimagruby/Cargo.toml index de12d3f9..664a87c4 100644 --- a/libimagruby/Cargo.toml +++ b/libimagruby/Cargo.toml @@ -22,6 +22,7 @@ lazy_static = "0.2" log = "0.3" env_logger = "0.3" toml = "0.2" +uuid = "0.3" [dependencies.libimagerror] path = "../libimagerror" diff --git a/libimagruby/src/cache.rs b/libimagruby/src/cache.rs index af49dad2..902c0545 100644 --- a/libimagruby/src/cache.rs +++ b/libimagruby/src/cache.rs @@ -21,41 +21,15 @@ use std::collections::BTreeMap; use std::sync::Arc; use std::sync::Mutex; -use libimagstore::store::FileLockEntry; -use libimagstore::storeid::StoreId; +use uuid::Uuid; + +use libimagstore::store::Store; + +#[derive(Clone, Debug, Ord, Hash, Eq, PartialOrd, PartialEq)] +pub struct StoreHandle(Uuid); lazy_static! { - /// A cache for FileLockEntry objects. - /// - /// # Why do we need this? - /// - /// "ruru" does us give the ability to move objects to the Ruby namespace by wrapping them into - /// a struct that can be handled by the Ruby VM. This is awesome. - /// Although, we cannot get the Object back into the Rust namespace (at least not as `Object`, - /// only as `&mut Object`). - /// - /// Now, have a look at `Store::save_as()` for example. It has the type signature - /// `fn save_as(&self, FileLockEntry, StoreId) -> Result<()>;`. - /// This means that we need to _move_ a `FileLockEntry` into the Store. - /// - /// But we cannot, if the FileLockEntry is in the Ruby namespace (we cannot get it back). - /// - /// The solution is simple and complex at the same time: Do not move any object into the Ruby - /// namespace! - /// - /// What we do: If the Ruby code wants us to get a `FileLockEntry`, we actually move the - /// `FileLockEntry` into this `FILE_LOCK_ENTRY_CACHE` and give the Ruby process a `Handle` for - /// the `FileLockEntry` object. - /// - /// From the Ruby world, it looks like a `FileLockEntry`, but it is not. The implementations in - /// this very library fetch the `FileLockEntry` from this cache and operate on it, putting it - /// back into this cache afterwards. - /// - /// # Performance? - /// - /// I don't care right now. It is Ruby, it is slow anyways. If it works, I don't mind. And I - /// don't see why we couldn't change this implementation later under the hood... - pub static ref FILE_LOCK_ENTRY_CACHE: Arc>>> = { + pub static ref RUBY_STORE_CACHE: Arc>> = { Arc::new(Mutex::new(BTreeMap::new())) }; } diff --git a/libimagruby/src/entry.rs b/libimagruby/src/entry.rs index 8235818a..bc9906d6 100644 --- a/libimagruby/src/entry.rs +++ b/libimagruby/src/entry.rs @@ -23,6 +23,7 @@ use std::ops::Deref; use std::ops::DerefMut; use ruru::{Class, Object, AnyObject, Boolean, RString, VM, Hash, NilClass, VerifiedObject}; +use uuid::Uuid; use libimagstore::store::FileLockEntry as FLE; use libimagstore::store::EntryHeader; @@ -34,13 +35,22 @@ use ruby_utils::IntoToml; use toml_utils::IntoRuby; use util::Wrap; use util::Unwrap; -use cache::FILE_LOCK_ENTRY_CACHE; +use cache::RUBY_STORE_CACHE; +use cache::StoreHandle; -pub struct FileLockEntryHandle(StoreId); +pub struct FileLockEntryHandle(StoreHandle, StoreId); impl FileLockEntryHandle { - pub fn new(id: StoreId) -> FileLockEntryHandle { - FileLockEntryHandle(id) + pub fn new(sh: StoreHandle, id: StoreId) -> FileLockEntryHandle { + FileLockEntryHandle(sh, id) + } + + pub fn store_handle(&self) -> &StoreHandle { + &self.0 + } + + pub fn fle_handle(&self) -> &StoreId { + &self.1 } } @@ -48,13 +58,13 @@ impl Deref for FileLockEntryHandle { type Target = StoreId; fn deref(&self) -> &Self::Target { - &self.0 + &self.1 } } impl DerefMut for FileLockEntryHandle { fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 + &mut self.1 } } @@ -64,21 +74,21 @@ impl_unwrap!(RFileLockEntry, FileLockEntryHandle, FLE_WRAPPER); impl_wrap!(FileLockEntryHandle, FLE_WRAPPER); impl_verified_object!(RFileLockEntry); -/// Helper macro for operating on FILE_LOCK_ENTRY_CACHE object +/// Helper macro for operating on RUBY_STORE_CACHE object /// -/// This macro fetches an ARC from the FILE_LOCK_ENTRY_CACHE, then locks the Mutex inside it +/// This macro fetches an ARC from the RUBY_STORE_CACHE, then locks the Mutex inside it /// and calls the $operation on the element inside the Mutex, this synchronizing the -/// operation on the FILE_LOCK_ENTRY_CACHE. +/// operation on the RUBY_STORE_CACHE. /// /// Yes, this is performance-wise not very elegant, but we're working with Ruby, and we need -/// to cache objects (why, see documentation for FILE_LOCK_ENTRY_CACHE). +/// to cache objects (why, see documentation for RUBY_STORE_CACHE). /// #[macro_export] macro_rules! operate_on_fle_cache { (mut |$name: ident| $operation: block) => {{ - use cache::FILE_LOCK_ENTRY_CACHE; + use cache::RUBY_STORE_CACHE; - let arc = FILE_LOCK_ENTRY_CACHE.clone(); + let arc = RUBY_STORE_CACHE.clone(); { let lock = arc.lock(); match lock { @@ -91,9 +101,9 @@ macro_rules! operate_on_fle_cache { } }}; (|$name: ident| $operation: block) => {{ - use cache::FILE_LOCK_ENTRY_CACHE; + use cache::RUBY_STORE_CACHE; - let arc = FILE_LOCK_ENTRY_CACHE.clone(); + let arc = RUBY_STORE_CACHE.clone(); { let lock = arc.lock(); match lock { @@ -114,8 +124,23 @@ methods!( fn r_get_location() -> AnyObject { operate_on_fle_cache!(|hm| { - match hm.get(itself.get_data(&*FLE_WRAPPER)) { - Some(el) => el.get_location().clone().wrap(), + let handle = itself.get_data(&*FLE_WRAPPER); + match hm.get(handle.store_handle()) { + Some(store) => { + match store.get(handle.fle_handle().clone()) { + Ok(Some(mut fle)) => { + fle.get_location().clone().wrap() + }, + Ok(None) => { + VM::raise(Class::from_existing("RuntimeError"), "Obj does not exist"); + NilClass::new().to_any_object() + }, + Err(e) => { + VM::raise(Class::from_existing("RuntimeError"), e.description()); + NilClass::new().to_any_object() + }, + } + }, None => { VM::raise(Class::from_existing("RuntimeError"), "Tried to operate on non-existing object"); @@ -127,8 +152,23 @@ methods!( fn r_get_header() -> AnyObject { operate_on_fle_cache!(|hm| { - match hm.get(itself.get_data(&*FLE_WRAPPER)) { - Some(el) => el.get_header().clone().wrap(), + let handle = itself.get_data(&*FLE_WRAPPER); + match hm.get(handle.store_handle()) { + Some(store) => { + match store.get(handle.fle_handle().clone()) { + Ok(Some(mut fle)) => { + fle.get_header().clone().wrap() + }, + Ok(None) => { + VM::raise(Class::from_existing("RuntimeError"), "Obj does not exist"); + NilClass::new().to_any_object() + }, + Err(e) => { + VM::raise(Class::from_existing("RuntimeError"), e.description()); + NilClass::new().to_any_object() + }, + } + }, None => { VM::raise(Class::from_existing("RuntimeError"), "Tried to operate on non-existing object"); @@ -153,9 +193,20 @@ methods!( }; operate_on_fle_cache!(mut |hm| { - match hm.get_mut(itself.get_data(&*FLE_WRAPPER)) { - Some(mut el) => { - *el.get_header_mut() = entryheader; + let handle = itself.get_data(&*FLE_WRAPPER); + match hm.get_mut(handle.store_handle()) { + Some(mut store) => { + match store.get(handle.fle_handle().clone()) { + Ok(Some(mut fle)) => { + *fle.get_header_mut() = entryheader; + }, + Ok(None) => { + VM::raise(Class::from_existing("RuntimeError"), "Obj does not exist"); + }, + Err(e) => { + VM::raise(Class::from_existing("RuntimeError"), e.description()); + }, + } }, None => { VM::raise(Class::from_existing("RuntimeError"), @@ -170,8 +221,23 @@ methods!( fn r_get_content() -> AnyObject { operate_on_fle_cache!(|hm| { - match hm.get(itself.get_data(&*FLE_WRAPPER)) { - Some(el) => el.get_content().clone().wrap(), + let handle = itself.get_data(&*FLE_WRAPPER); + match hm.get(handle.store_handle()) { + Some(store) => { + match store.get(handle.fle_handle().clone()) { + Ok(Some(mut fle)) => { + fle.get_content().clone().wrap() + }, + Ok(None) => { + VM::raise(Class::from_existing("RuntimeError"), "Obj does not exist"); + NilClass::new().to_any_object() + } + Err(e) => { + VM::raise(Class::from_existing("RuntimeError"), e.description()); + NilClass::new().to_any_object() + }, + } + } None => NilClass::new().to_any_object() } }) @@ -192,10 +258,21 @@ methods!( }; operate_on_fle_cache!(mut |hm| { - match hm.get_mut(itself.get_data(&*FLE_WRAPPER)) { - Some(el) => { - *el.get_content_mut() = content; - }, + let handle = itself.get_data(&*FLE_WRAPPER); + match hm.get_mut(handle.store_handle()) { + Some(store) => { + match store.get(handle.fle_handle().clone()) { + Ok(Some(mut fle)) => { + *fle.get_content_mut() = content; + }, + Ok(None) => { + VM::raise(Class::from_existing("RuntimeError"), "Obj does not exist"); + } + Err(e) => { + VM::raise(Class::from_existing("RuntimeError"), e.description()); + }, + } + } None => { VM::raise(Class::from_existing("RuntimeError"), "Tried to operate on non-existing object"); diff --git a/libimagruby/src/lib.rs b/libimagruby/src/lib.rs index b2de3469..cb9c496b 100644 --- a/libimagruby/src/lib.rs +++ b/libimagruby/src/lib.rs @@ -22,6 +22,7 @@ #[macro_use] extern crate log; extern crate env_logger; extern crate toml; +extern crate uuid; #[macro_use] extern crate libimagerror; extern crate libimagstore;