From f0e10abc078a975c7a4f7f3081a2aeff2c0318d8 Mon Sep 17 00:00:00 2001 From: BingqingLyu Date: Mon, 29 May 2023 15:32:14 +0800 Subject: [PATCH] [GraphProxy] refine impl of LazyDetails --- .../src/adapters/csr_store/read_graph.rs | 139 +++++------------- .../src/adapters/exp_store/read_graph.rs | 120 ++++----------- .../src/adapters/gs_store/details.rs | 120 +++------------ 3 files changed, 82 insertions(+), 297 deletions(-) diff --git a/interactive_engine/executor/ir/graph_proxy/src/adapters/csr_store/read_graph.rs b/interactive_engine/executor/ir/graph_proxy/src/adapters/csr_store/read_graph.rs index eaf971e591d2..449db0f01abb 100644 --- a/interactive_engine/executor/ir/graph_proxy/src/adapters/csr_store/read_graph.rs +++ b/interactive_engine/executor/ir/graph_proxy/src/adapters/csr_store/read_graph.rs @@ -15,7 +15,6 @@ use std::convert::TryFrom; use std::fmt; -use std::sync::atomic::{AtomicPtr, Ordering}; use std::sync::Arc; use ahash::HashMap; @@ -255,24 +254,14 @@ struct LazyVertexDetails { // Specifically, Some(vec![]) indicates we need all properties // and None indicates we do not need any property prop_keys: Option>, - inner: AtomicPtr>, + inner: LocalVertex<'static, DefaultId, DefaultId>, } impl_as_any!(LazyVertexDetails); impl LazyVertexDetails { pub fn new(v: LocalVertex<'static, DefaultId, DefaultId>, prop_keys: Option>) -> Self { - let ptr = Box::into_raw(Box::new(v)); - LazyVertexDetails { prop_keys, inner: AtomicPtr::new(ptr) } - } - - fn get_vertex_ptr(&self) -> Option<*mut LocalVertex<'static, DefaultId, DefaultId>> { - let ptr = self.inner.load(Ordering::SeqCst); - if ptr.is_null() { - None - } else { - Some(ptr) - } + LazyVertexDetails { prop_keys, inner: v } } } @@ -288,20 +277,14 @@ impl fmt::Debug for LazyVertexDetails { impl Details for LazyVertexDetails { fn get_property(&self, key: &NameOrId) -> Option { if let NameOrId::Str(key) = key { - if let Some(ptr) = self.get_vertex_ptr() { - unsafe { - if key == "id" { - let mask = (1_usize << LABEL_SHIFT_BITS) - 1; - let original_id = ((*ptr).get_id() & mask) as i64; - Some(PropertyValue::Owned(Object::Primitive(Primitives::Long(original_id)))) - } else { - (*ptr) - .get_property(key) - .map(|prop| PropertyValue::Borrowed(to_borrow_object(prop))) - } - } + if key == "id" { + let mask = (1_usize << LABEL_SHIFT_BITS) - 1; + let original_id = (self.inner.get_id() & mask) as i64; + Some(PropertyValue::Owned(Object::Primitive(Primitives::Long(original_id)))) } else { - None + self.inner + .get_property(key) + .map(|prop| PropertyValue::Borrowed(to_borrow_object(prop))) } } else { info!("Have not support getting property by prop_id in exp_store yet"); @@ -311,28 +294,19 @@ impl Details for LazyVertexDetails { fn get_all_properties(&self) -> Option> { // the case of get_all_properties from vertex; - let props = if let Some(ptr) = self.get_vertex_ptr() { - unsafe { - if let Some(prop_key_vals) = (*ptr).get_all_properties() { - let mut all_props: HashMap = prop_key_vals - .into_iter() - .map(|(prop_key, prop_val)| (prop_key.into(), to_object(prop_val))) - .collect(); - let mask = (1_usize << LABEL_SHIFT_BITS) - 1; - let original_id = ((*ptr).get_id() & mask) as i64; - all_props.insert( - NameOrId::Str("id".to_string()), - Object::Primitive(Primitives::Long(original_id)), - ); - Some(all_props) - } else { - None - } - } + if let Some(prop_key_vals) = self.inner.get_all_properties() { + let mut all_props: HashMap = prop_key_vals + .into_iter() + .map(|(prop_key, prop_val)| (prop_key.into(), to_object(prop_val))) + .collect(); + let mask = (1_usize << LABEL_SHIFT_BITS) - 1; + let original_id = (self.inner.get_id() & mask) as i64; + all_props + .insert(NameOrId::Str("id".to_string()), Object::Primitive(Primitives::Long(original_id))); + Some(all_props) } else { None - }; - props + } } fn get_property_keys(&self) -> Option> { @@ -340,17 +314,6 @@ impl Details for LazyVertexDetails { } } -impl Drop for LazyVertexDetails { - fn drop(&mut self) { - let ptr = self.inner.load(Ordering::SeqCst); - if !ptr.is_null() { - unsafe { - std::ptr::drop_in_place(ptr); - } - } - } -} - /// LazyEdgeDetails is used for local property fetching optimization. /// That is, the required properties will not be materialized until LazyEdgeDetails need to be shuffled. #[allow(dead_code)] @@ -360,24 +323,14 @@ struct LazyEdgeDetails { // Specifically, Some(vec![]) indicates we need all properties // and None indicates we do not need any property, prop_keys: Option>, - inner: AtomicPtr>, + inner: LocalEdge<'static, DefaultId, DefaultId>, } impl_as_any!(LazyEdgeDetails); impl LazyEdgeDetails { pub fn new(e: LocalEdge<'static, DefaultId, DefaultId>, prop_keys: Option>) -> Self { - let ptr = Box::into_raw(Box::new(e)); - LazyEdgeDetails { prop_keys, inner: AtomicPtr::new(ptr) } - } - - fn get_edge_ptr(&self) -> Option<*mut LocalEdge<'static, DefaultId, DefaultId>> { - let ptr = self.inner.load(Ordering::SeqCst); - if ptr.is_null() { - None - } else { - Some(ptr) - } + LazyEdgeDetails { prop_keys, inner: e } } } @@ -385,7 +338,7 @@ impl fmt::Debug for LazyEdgeDetails { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("LazyEdgeDetails") .field("prop_keys", &self.prop_keys) - .field("inner", &self.inner) + // .field("inner", &self.inner) .finish() } } @@ -393,16 +346,9 @@ impl fmt::Debug for LazyEdgeDetails { impl Details for LazyEdgeDetails { fn get_property(&self, key: &NameOrId) -> Option { if let NameOrId::Str(key) = key { - let ptr = self.get_edge_ptr(); - if let Some(ptr) = ptr { - unsafe { - (*ptr) - .get_property(key) - .map(|prop| PropertyValue::Borrowed(to_borrow_object(prop))) - } - } else { - None - } + self.inner + .get_property(key) + .map(|prop| PropertyValue::Borrowed(to_borrow_object(prop))) } else { info!("Have not support getting property by prop_id in experiments store yet"); None @@ -410,23 +356,15 @@ impl Details for LazyEdgeDetails { } fn get_all_properties(&self) -> Option> { - // the case of get_all_properties from vertex; - let props = if let Some(ptr) = self.get_edge_ptr() { - unsafe { - if let Some(prop_key_vals) = (*ptr).get_all_properties() { - let all_props: HashMap = prop_key_vals - .into_iter() - .map(|(prop_key, prop_val)| (prop_key.into(), to_object(prop_val))) - .collect(); - Some(all_props) - } else { - None - } - } + if let Some(prop_key_vals) = self.inner.get_all_properties() { + let all_props: HashMap = prop_key_vals + .into_iter() + .map(|(prop_key, prop_val)| (prop_key.into(), to_object(prop_val))) + .collect(); + Some(all_props) } else { None - }; - props + } } fn get_property_keys(&self) -> Option> { @@ -434,17 +372,6 @@ impl Details for LazyEdgeDetails { } } -impl Drop for LazyEdgeDetails { - fn drop(&mut self) { - let ptr = self.inner.load(Ordering::SeqCst); - if !ptr.is_null() { - unsafe { - std::ptr::drop_in_place(ptr); - } - } - } -} - #[inline] fn to_object<'a>(ref_item: RefItem<'a>) -> Object { match ref_item { diff --git a/interactive_engine/executor/ir/graph_proxy/src/adapters/exp_store/read_graph.rs b/interactive_engine/executor/ir/graph_proxy/src/adapters/exp_store/read_graph.rs index e05c2c6abe85..9b3c186c1e59 100644 --- a/interactive_engine/executor/ir/graph_proxy/src/adapters/exp_store/read_graph.rs +++ b/interactive_engine/executor/ir/graph_proxy/src/adapters/exp_store/read_graph.rs @@ -15,7 +15,6 @@ use std::fmt; use std::path::Path; -use std::sync::atomic::{AtomicPtr, Ordering}; use std::sync::Arc; use ahash::HashMap; @@ -434,24 +433,14 @@ struct LazyVertexDetails { // Specifically, Some(vec![]) indicates we need all properties // and None indicates we do not need any property prop_keys: Option>, - inner: AtomicPtr>, + inner: LocalVertex<'static, DefaultId>, } impl_as_any!(LazyVertexDetails); impl LazyVertexDetails { pub fn new(v: LocalVertex<'static, DefaultId>, prop_keys: Option>) -> Self { - let ptr = Box::into_raw(Box::new(v)); - LazyVertexDetails { prop_keys, inner: AtomicPtr::new(ptr) } - } - - fn get_vertex_ptr(&self) -> Option<*mut LocalVertex<'static, DefaultId>> { - let ptr = self.inner.load(Ordering::SeqCst); - if ptr.is_null() { - None - } else { - Some(ptr) - } + LazyVertexDetails { prop_keys, inner: v } } } @@ -467,15 +456,9 @@ impl fmt::Debug for LazyVertexDetails { impl Details for LazyVertexDetails { fn get_property(&self, key: &NameOrId) -> Option { if let NameOrId::Str(key) = key { - if let Some(ptr) = self.get_vertex_ptr() { - unsafe { - (*ptr) - .get_property(key) - .map(|prop| PropertyValue::Borrowed(prop)) - } - } else { - None - } + self.inner + .get_property(key) + .map(|prop| PropertyValue::Borrowed(prop)) } else { info!("Have not support getting property by prop_id in exp_store yet"); None @@ -483,20 +466,14 @@ impl Details for LazyVertexDetails { } fn get_all_properties(&self) -> Option> { - if let Some(ptr) = self.get_vertex_ptr() { - unsafe { - (*ptr) - .clone_all_properties() - .map(|prop_key_vals| { - prop_key_vals - .into_iter() - .map(|(prop_key, prop_val)| (prop_key.into(), prop_val as Object)) - .collect() - }) - } - } else { - None - } + self.inner + .clone_all_properties() + .map(|prop_key_vals| { + prop_key_vals + .into_iter() + .map(|(prop_key, prop_val)| (prop_key.into(), prop_val as Object)) + .collect() + }) } fn get_property_keys(&self) -> Option> { @@ -504,17 +481,6 @@ impl Details for LazyVertexDetails { } } -impl Drop for LazyVertexDetails { - fn drop(&mut self) { - let ptr = self.inner.load(Ordering::SeqCst); - if !ptr.is_null() { - unsafe { - std::ptr::drop_in_place(ptr); - } - } - } -} - /// LazyEdgeDetails is used for local property fetching optimization. /// That is, the required properties will not be materialized until LazyEdgeDetails need to be shuffled. #[allow(dead_code)] @@ -524,24 +490,14 @@ struct LazyEdgeDetails { // Specifically, Some(vec![]) indicates we need all properties // and None indicates we do not need any property, prop_keys: Option>, - inner: AtomicPtr>, + inner: LocalEdge<'static, DefaultId, InternalId>, } impl_as_any!(LazyEdgeDetails); impl LazyEdgeDetails { pub fn new(e: LocalEdge<'static, DefaultId, InternalId>, prop_keys: Option>) -> Self { - let ptr = Box::into_raw(Box::new(e)); - LazyEdgeDetails { prop_keys, inner: AtomicPtr::new(ptr) } - } - - fn get_edge_ptr(&self) -> Option<*mut LocalEdge<'static, DefaultId, InternalId>> { - let ptr = self.inner.load(Ordering::SeqCst); - if ptr.is_null() { - None - } else { - Some(ptr) - } + LazyEdgeDetails { prop_keys, inner: e } } } @@ -557,16 +513,9 @@ impl fmt::Debug for LazyEdgeDetails { impl Details for LazyEdgeDetails { fn get_property(&self, key: &NameOrId) -> Option { if let NameOrId::Str(key) = key { - let ptr = self.get_edge_ptr(); - if let Some(ptr) = ptr { - unsafe { - (*ptr) - .get_property(key) - .map(|prop| PropertyValue::Borrowed(prop)) - } - } else { - None - } + self.inner + .get_property(key) + .map(|prop| PropertyValue::Borrowed(prop)) } else { info!("Have not support getting property by prop_id in experiments store yet"); None @@ -574,20 +523,14 @@ impl Details for LazyEdgeDetails { } fn get_all_properties(&self) -> Option> { - if let Some(ptr) = self.get_edge_ptr() { - unsafe { - (*ptr) - .clone_all_properties() - .map(|prop_key_vals| { - prop_key_vals - .into_iter() - .map(|(prop_key, prop_val)| (prop_key.into(), prop_val as Object)) - .collect() - }) - } - } else { - None - } + self.inner + .clone_all_properties() + .map(|prop_key_vals| { + prop_key_vals + .into_iter() + .map(|(prop_key, prop_val)| (prop_key.into(), prop_val as Object)) + .collect() + }) } fn get_property_keys(&self) -> Option> { @@ -595,17 +538,6 @@ impl Details for LazyEdgeDetails { } } -impl Drop for LazyEdgeDetails { - fn drop(&mut self) { - let ptr = self.inner.load(Ordering::SeqCst); - if !ptr.is_null() { - unsafe { - std::ptr::drop_in_place(ptr); - } - } - } -} - /// Edge's ID is encoded by its internal index #[inline] fn encode_runtime_e_id(e: &LocalEdge) -> ID { diff --git a/interactive_engine/executor/ir/graph_proxy/src/adapters/gs_store/details.rs b/interactive_engine/executor/ir/graph_proxy/src/adapters/gs_store/details.rs index 89c513677c85..909f7189644e 100644 --- a/interactive_engine/executor/ir/graph_proxy/src/adapters/gs_store/details.rs +++ b/interactive_engine/executor/ir/graph_proxy/src/adapters/gs_store/details.rs @@ -14,7 +14,6 @@ //! limitations under the License. use std::fmt; -use std::sync::atomic::{AtomicPtr, Ordering}; use ahash::HashMap; use dyn_type::Object; @@ -69,7 +68,7 @@ where // Specifically, in graphscope store, None means we do not need any property, // and Some(vec![]) means we need all properties prop_keys: Option>, - inner: AtomicPtr, + inner: V, } impl LazyVertexDetails @@ -77,17 +76,7 @@ where V: StoreVertex + 'static, { pub fn new(v: V, prop_keys: Option>) -> Self { - let ptr = Box::into_raw(Box::new(v)); - LazyVertexDetails { prop_keys, inner: AtomicPtr::new(ptr) } - } - - fn get_vertex_ptr(&self) -> Option<*mut V> { - let ptr = self.inner.load(Ordering::SeqCst); - if ptr.is_null() { - None - } else { - Some(ptr) - } + LazyVertexDetails { prop_keys, inner: v } } } @@ -98,7 +87,7 @@ where fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("gs_store LazyVertexDetails") .field("properties", &self.prop_keys) - .field("inner", &self.inner) + // .field("inner", &self.inner) .finish() } } @@ -109,15 +98,9 @@ where { fn get_property(&self, key: &NameOrId) -> Option { if let NameOrId::Id(key) = key { - if let Some(ptr) = self.get_vertex_ptr() { - unsafe { - (*ptr) - .get_property(*key as PropId) - .map(|prop| PropertyValue::Owned(encode_runtime_prop_val(prop))) - } - } else { - None - } + self.inner + .get_property(*key as PropId) + .map(|prop| PropertyValue::Owned(encode_runtime_prop_val(prop))) } else { info!("Have not support getting property by prop_name in gs_store yet"); None @@ -125,18 +108,12 @@ where } fn get_all_properties(&self) -> Option> { - if let Some(ptr) = self.get_vertex_ptr() { - unsafe { - Some( - (*ptr) - .get_properties() - .map(|(prop_id, prop_val)| encode_runtime_property(prop_id, prop_val)) - .collect(), - ) - } - } else { - None - } + Some( + self.inner + .get_properties() + .map(|(prop_id, prop_val)| encode_runtime_property(prop_id, prop_val)) + .collect(), + ) } fn get_property_keys(&self) -> Option> { @@ -157,20 +134,6 @@ where } } -impl Drop for LazyVertexDetails -where - V: StoreVertex + 'static, -{ - fn drop(&mut self) { - let ptr = self.inner.load(Ordering::SeqCst); - if !ptr.is_null() { - unsafe { - std::ptr::drop_in_place(ptr); - } - } - } -} - /// LazyEdgeDetails is used for local property fetching optimization. /// That is, the required properties will not be materialized until LazyEdgeDetails need to be shuffled. #[allow(dead_code)] @@ -183,7 +146,7 @@ where // Specifically, in graphscope store, None means we do not need any property, // and Some(vec![]) means we need all properties prop_keys: Option>, - inner: AtomicPtr, + inner: E, } impl LazyEdgeDetails @@ -191,17 +154,7 @@ where E: StoreEdge + 'static, { pub fn new(e: E, prop_keys: Option>) -> Self { - let ptr = Box::into_raw(Box::new(e)); - LazyEdgeDetails { prop_keys, inner: AtomicPtr::new(ptr) } - } - - fn get_edge_ptr(&self) -> Option<*mut E> { - let ptr = self.inner.load(Ordering::SeqCst); - if ptr.is_null() { - None - } else { - Some(ptr) - } + LazyEdgeDetails { prop_keys, inner: e } } } @@ -212,7 +165,6 @@ where fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("gs_store LazyEdgeDetails") .field("properties", &self.prop_keys) - .field("inner", &self.inner) .finish() } } @@ -223,15 +175,9 @@ where { fn get_property(&self, key: &NameOrId) -> Option { if let NameOrId::Id(key) = key { - if let Some(ptr) = self.get_edge_ptr() { - unsafe { - (*ptr) - .get_property(*key as PropId) - .map(|prop| PropertyValue::Owned(encode_runtime_prop_val(prop))) - } - } else { - None - } + self.inner + .get_property(*key as PropId) + .map(|prop| PropertyValue::Owned(encode_runtime_prop_val(prop))) } else { info!("Have not support getting property by prop_name in gs_store yet"); None @@ -240,18 +186,12 @@ where fn get_all_properties(&self) -> Option> { // the case of get_all_properties from vertex; - if let Some(ptr) = self.get_edge_ptr() { - unsafe { - Some( - (*ptr) - .get_properties() - .map(|(prop_id, prop_val)| encode_runtime_property(prop_id, prop_val)) - .collect(), - ) - } - } else { - None - } + Some( + self.inner + .get_properties() + .map(|(prop_id, prop_val)| encode_runtime_property(prop_id, prop_val)) + .collect(), + ) } fn get_property_keys(&self) -> Option> { @@ -271,17 +211,3 @@ where self } } - -impl Drop for LazyEdgeDetails -where - E: StoreEdge + 'static, -{ - fn drop(&mut self) { - let ptr = self.inner.load(Ordering::SeqCst); - if !ptr.is_null() { - unsafe { - std::ptr::drop_in_place(ptr); - } - } - } -}