Merge pull request #307 from korpling/feature/remove-whole-item-from-…

…annostorage

Add "remove_item" function to annotation storages

CHANGELOG.md

@@ -5,6 +5,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- New method `remove_item()` for annotation storages that allows for more
+  efficient removal if not only a single annotation, but the whole item should
+  be deleted. This is used in when applying a `DeleteNode` or `DeleteEdge`
+  event.
+
 ## [3.4.0] - 2024-08-20
 
 ### Added

core/src/annostorage/inmemory.rs

@@ -11,7 +11,7 @@ use rustc_hash::FxHashSet;
 use smartstring::alias::String;
 use smartstring::{LazyCompact, SmartString};
 use std::borrow::Cow;
-use std::collections::{BTreeMap, HashMap};
+use std::collections::{BTreeMap, BTreeSet, HashMap};
 use std::hash::Hash;
 use std::path::Path;
 use std::sync::Arc;
@@ -22,7 +22,7 @@ struct SparseAnnotation {
     val: usize,
 }
 
-type ValueItemMap<T> = HashMap<usize, Vec<T>>;
+type ValueItemMap<T> = HashMap<usize, BTreeSet<T>>;
 
 #[derive(Serialize, Deserialize, Clone, Default)]
 pub struct AnnoStorageImpl<T: Ord + Hash + Default> {
@@ -86,7 +86,7 @@ impl<T: Ord + Hash + Clone + serde::Serialize + serde::de::DeserializeOwned + De
     fn remove_element_from_by_anno(&mut self, anno: &SparseAnnotation, item: &T) {
         let remove_anno_key = if let Some(annos_for_key) = self.by_anno.get_mut(&anno.key) {
             let remove_anno_val = if let Some(items_for_anno) = annos_for_key.get_mut(&anno.val) {
-                items_for_anno.retain(|i| i != item);
+                items_for_anno.remove(item);
                 items_for_anno.is_empty()
             } else {
                 false
@@ -230,12 +230,13 @@ where
 
         // inserts a new relation between the annotation and the item
         // if set is not existing yet it is created
-        self.by_anno
+        let item_list_for_value = self
+            .by_anno
             .entry(anno.key)
             .or_default()
             .entry(anno.val)
-            .or_default()
-            .push(item.clone());
+            .or_default();
+        item_list_for_value.insert(item.clone());
 
         if existing_anno.is_none() {
             // a new annotation entry was inserted and did not replace an existing one
@@ -307,6 +308,40 @@ where
         Ok(result)
     }
 
+    fn remove_item(&mut self, item: &T) -> Result<bool> {
+        let mut result = false;
+
+        if let Some(all_annos) = self.by_container.remove(item) {
+            for anno in all_annos {
+                // since value was found, also remove the item from the other containers
+                self.remove_element_from_by_anno(&anno, item);
+
+                if let Some(resolved_key) = self.anno_keys.get_value(anno.key) {
+                    // decrease the annotation count for this key
+                    let new_key_count: usize =
+                        if let Some(num_of_keys) = self.anno_key_sizes.get_mut(&resolved_key) {
+                            *num_of_keys -= 1;
+                            *num_of_keys
+                        } else {
+                            0
+                        };
+                    // if annotation count dropped to zero remove the key
+                    if new_key_count == 0 {
+                        self.by_anno.remove(&anno.key);
+                        self.anno_key_sizes.remove(&resolved_key);
+                        self.anno_keys.remove(anno.key);
+                    }
+                }
+
+                result = true;
+                self.check_and_remove_value_symbol(anno.val);
+                self.total_number_of_annos -= 1;
+            }
+        }
+
+        Ok(result)
+    }
+
     fn clear(&mut self) -> Result<()> {
         self.clear_internal();
         Ok(())

core/src/annostorage/inmemory/tests.rs

@@ -104,6 +104,29 @@ fn remove() {
     assert_eq!(&0, a.anno_key_sizes.get(&test_anno.key).unwrap_or(&0));
 }
 
+#[test]
+fn remove_item() {
+    let test_anno = Annotation {
+        key: AnnoKey {
+            name: "anno1".into(),
+            ns: "annis1".into(),
+        },
+        val: "test".into(),
+    };
+
+    let mut a: AnnoStorageImpl<NodeID> = AnnoStorageImpl::new();
+    a.insert(1, test_anno.clone()).unwrap();
+
+    assert_eq!(1, a.number_of_annotations().unwrap());
+    assert_eq!(1, a.anno_key_sizes.len());
+    assert_eq!(&1, a.anno_key_sizes.get(&test_anno.key).unwrap());
+
+    a.remove_item(&1).unwrap();
+
+    assert_eq!(0, a.number_of_annotations().unwrap());
+    assert_eq!(&0, a.anno_key_sizes.get(&test_anno.key).unwrap_or(&0));
+}
+
 #[test]
 fn get_node_id_from_name() {
     let key = NODE_NAME_KEY.as_ref().clone();

core/src/annostorage/mod.rs

@@ -187,6 +187,10 @@ where
     /// Returns the value for that annotation, if it existed.
     fn remove_annotation_for_item(&mut self, item: &T, key: &AnnoKey) -> Result<Option<Cow<str>>>;
 
+    /// Remove all annotations for the given item. Returns whether the item had
+    /// any annotations.
+    fn remove_item(&mut self, item: &T) -> Result<bool>;
+
     /// Remove all annotations.
     fn clear(&mut self) -> Result<()>;
 

core/src/annostorage/ondisk.rs

@@ -411,6 +411,57 @@ where
         Ok(result)
     }
 
+    fn remove_item(&mut self, item: &T) -> Result<bool> {
+        let mut result = false;
+
+        let start = create_by_container_key(item.clone(), usize::MIN);
+        let end = create_by_container_key(item.clone(), usize::MAX);
+
+        let annos_in_container: Result<Vec<_>> = self.by_container.range(start..=end).collect();
+        let annos_in_container = annos_in_container?;
+
+        for (by_container_key, _) in annos_in_container {
+            let symbol_id = usize::parse_key(&by_container_key[T::key_size()..])?;
+            let key = self
+                .anno_key_symbols
+                .get_value(symbol_id)
+                .unwrap_or_default();
+
+            if let Some(val) = self.by_container.remove(&by_container_key)? {
+                // remove annotation from by_anno_qname
+                let anno = Annotation {
+                    key: key.as_ref().clone(),
+                    val: val.into(),
+                };
+
+                self.by_anno_qname.remove(&create_by_anno_qname_key(
+                    item.clone(),
+                    symbol_id,
+                    &anno.val,
+                ))?;
+                // decrease the annotation count for this key
+                let new_key_count: usize =
+                    if let Some(num_of_keys) = self.anno_key_sizes.get_mut(&key) {
+                        *num_of_keys -= 1;
+                        *num_of_keys
+                    } else {
+                        0
+                    };
+                // if annotation count dropped to zero remove the key
+                if new_key_count == 0 {
+                    self.anno_key_sizes.remove(&key);
+                    if let Some(id) = self.anno_key_symbols.get_symbol(&key) {
+                        self.anno_key_symbols.remove(id);
+                    }
+                }
+
+                result = true;
+            }
+        }
+
+        Ok(result)
+    }
+
     fn remove_annotation_for_item(&mut self, item: &T, key: &AnnoKey) -> Result<Option<Cow<str>>> {
         // remove annotation from by_container
         if let Some(symbol_id) = self.anno_key_symbols.get_symbol(key) {

core/src/annostorage/ondisk/tests.rs

@@ -112,6 +112,30 @@ fn remove() {
     assert_eq!(&0, a.anno_key_sizes.get(&test_anno.key).unwrap_or(&0));
 }
 
+#[test]
+fn remove_item() {
+    LOGGER_INIT.call_once(env_logger::init);
+    let test_anno = Annotation {
+        key: AnnoKey {
+            name: "anno1".into(),
+            ns: "annis1".into(),
+        },
+        val: "test".into(),
+    };
+
+    let mut a = AnnoStorageImpl::new(None).unwrap();
+    a.insert(1, test_anno.clone()).unwrap();
+
+    assert_eq!(1, a.number_of_annotations().unwrap());
+    assert_eq!(1, a.anno_key_sizes.len());
+    assert_eq!(&1, a.anno_key_sizes.get(&test_anno.key).unwrap());
+
+    a.remove_item(&1).unwrap();
+
+    assert_eq!(0, a.number_of_annotations().unwrap());
+    assert_eq!(&0, a.anno_key_sizes.get(&test_anno.key).unwrap_or(&0));
+}
+
 #[test]
 fn get_node_id_from_name() {
     let key = NODE_NAME_KEY.as_ref().clone();

core/src/graph/mod.rs

@@ -412,15 +412,8 @@ impl<CT: ComponentType> Graph<CT> {
                         &mut node_id_cache,
                     )? {
                         // delete all annotations
-                        {
-                            for a in self
-                                .node_annos
-                                .get_annotations_for_item(&existing_node_id)?
-                            {
-                                self.node_annos
-                                    .remove_annotation_for_item(&existing_node_id, &a.key)?;
-                            }
-                        }
+                        self.node_annos.remove_item(&existing_node_id)?;
+
                         // delete all edges pointing to this node either as source or target
                         for c in all_components.iter() {
                             if let Ok(gs) = self.get_or_create_writable(c) {

core/src/graph/storage/adjacencylist.rs

@@ -288,10 +288,7 @@ impl WriteableGraphStorage for AdjacencyListStorage {
                 ingoing.remove(idx);
             }
         }
-        let annos = self.annos.get_annotations_for_item(edge)?;
-        for a in annos {
-            self.annos.remove_annotation_for_item(edge, &a.key)?;
-        }
+        self.annos.remove_item(edge)?;
 
         Ok(())
     }

core/src/graph/storage/disk_adjacency.rs

@@ -323,10 +323,7 @@ impl WriteableGraphStorage for DiskAdjacencyListStorage {
         self.edges.remove(edge)?;
         self.inverse_edges.remove(&edge.inverse())?;
 
-        let annos = self.annos.get_annotations_for_item(edge)?;
-        for a in annos {
-            self.annos.remove_annotation_for_item(edge, &a.key)?;
-        }
+        self.annos.remove_item(edge)?;
 
         Ok(())
     }