Add locking to node and edge calls.

Files affected: node.cc, edge.cc, edge.h, graph.cc, GraphImpl.h, AvlTreeIndex.cc, AvlTreeIndex.h, Index.cc, Index.h, IndexManager.cc, TransactionImpl.h
IntelLabs · Dec 21, 2018 · 9dd32ab · 9dd32ab
1 parent abe0255
commit 9dd32ab
Show file tree

Hide file tree

Showing 11 changed files with 204 additions and 66 deletions.
diff --git a/include/edge.h b/include/edge.h
@@ -52,9 +52,9 @@ namespace PMGD {
         ~Edge() = delete;
         void operator=(const Edge &) = delete;
         EdgeID get_id() const;
-        StringID get_tag() const { return _tag; }
-        Node &get_source() const { return *_src; }
-        Node &get_destination() const { return *_dest; }
+        StringID get_tag() const;
+        Node &get_source() const;
+        Node &get_destination() const;
         bool check_property(StringID property, Property &result) const;
         Property get_property(StringID property) const;
         PropertyIterator get_properties() const;

diff --git a/src/AvlTreeIndex.cc b/src/AvlTreeIndex.cc
@@ -382,7 +382,8 @@ namespace PMGD {
         using Index_IteratorImplBase<K>::_list_it; \
         using Index_IteratorImplBase<K>::_vacant_flag; \
         using Index_IteratorImplBase<K>::finish_init; \
-        using Index_IteratorImplBase<K>::_tx;
+        using Index_IteratorImplBase<K>::_tx; \
+        using Index_IteratorImplBase<K>::_index_type;
 
     // These iterator implementations are specific to instantiations
     // of AvlTreeIndex<K, V> with V = List<void *>.
@@ -399,6 +400,7 @@ namespace PMGD {
         ListTraverser<void *> _list_it;
         bool _vacant_flag = false;
         TransactionImpl *_tx;
+        Graph::IndexType _index_type;
 
         void finish_init() {
             if (!_path.empty()) {
@@ -455,13 +457,18 @@ namespace PMGD {
             return true;
         }
 
+        void set_index_type(Graph::IndexType index_type)
+          { _index_type = index_type; }
+
         void *ref() const
         {
             // _vacant_flag indicates that the object referred to by the
             // iterator has been removed from the index.
             if (EXPECT_FALSE(_vacant_flag))
                 throw PMGDException(VacantIterator);
-            return _list_it.ref();
+            void *value = _list_it.ref();
+            TransactionImpl::lock(_index_type, value, false);
+            return value;
         }
 
         void remove_notify(void *list_node)
@@ -686,19 +693,25 @@ namespace PMGD {
 }
 
 template <typename K, typename V>
-Index::Index_IteratorImplIntf *AvlTreeIndex<K,V>::get_iterator(bool reverse)
+Index::Index_IteratorImplIntf *AvlTreeIndex<K,V>::get_iterator(Graph::IndexType index_type, bool reverse)
 {
     // We can read lock the main index class here.
     TransactionImpl *tx = TransactionImpl::get_tx();
     tx->acquire_lock(TransactionImpl::IndexLock, this, false);
+    Index_IteratorImplBase<K> *impl = NULL;
+
     if (!reverse)
-        return new IndexRangeNomax_IteratorImpl<K>(this);
+        impl = new IndexRangeNomax_IteratorImpl<K>(this);
     else
-        return new IndexRangeNomin_IteratorImpl<K>(this);
+        impl =  new IndexRangeNomin_IteratorImpl<K>(this);
+
+    impl->set_index_type(index_type);
+    return impl;
 }
 
 template <typename K, typename V>
-Index::Index_IteratorImplIntf *AvlTreeIndex<K,V>::get_iterator(const K &key, PropertyPredicate::Op op, bool reverse)
+Index::Index_IteratorImplIntf *AvlTreeIndex<K,V>::get_iterator(Graph::IndexType index_type, const K &key,
+                                                    PropertyPredicate::Op op, bool reverse)
 {
     // We can read lock the main index class here.
     TransactionImpl *tx = TransactionImpl::get_tx();
@@ -746,13 +759,14 @@ Index::Index_IteratorImplIntf *AvlTreeIndex<K,V>::get_iterator(const K &key, Pro
             break;
     }
 
+    impl->set_index_type(index_type);
     return impl;
 }
 
 template <typename K, typename V>
-Index::Index_IteratorImplIntf *AvlTreeIndex<K,V>::get_iterator(const K &min, const K& max,
-                                          PropertyPredicate::Op op,
-                                          bool reverse)
+Index::Index_IteratorImplIntf *AvlTreeIndex<K,V>::get_iterator(Graph::IndexType index_type, const K &min,
+                                                               const K& max, PropertyPredicate::Op op,
+                                                               bool reverse)
 {
     bool incl_min = true, incl_max = true;
 
@@ -767,10 +781,15 @@ Index::Index_IteratorImplIntf *AvlTreeIndex<K,V>::get_iterator(const K &min, con
 
     TransactionImpl *tx = TransactionImpl::get_tx();
     tx->acquire_lock(TransactionImpl::IndexLock, this, false);
+    Index_IteratorImplBase<K> *impl = NULL;
+
     if (!reverse)
-        return new IndexRange_IteratorImpl<K>(this, min, max, incl_min, incl_max);
+        impl =  new IndexRange_IteratorImpl<K>(this, min, max, incl_min, incl_max);
     else
-        return new IndexRangeReverse_IteratorImpl<K>(this, min, max, incl_min, incl_max);
+        impl =  new IndexRangeReverse_IteratorImpl<K>(this, min, max, incl_min, incl_max);
+
+    impl->set_index_type(index_type);
+    return impl;
 }
 
 template <typename K, typename V>

diff --git a/src/AvlTreeIndex.h b/src/AvlTreeIndex.h
@@ -96,10 +96,12 @@ namespace PMGD {
         using AvlTree<K,V>::add;
         using AvlTree<K,V>::remove;
 
-        Index::Index_IteratorImplIntf *get_iterator(bool reverse);
-        Index::Index_IteratorImplIntf *get_iterator(const K &key, PropertyPredicate::Op op, bool reverse);
-        Index::Index_IteratorImplIntf *get_iterator(const K &min, const K &max, PropertyPredicate::Op op,
-                               bool reverse);
+        Index::Index_IteratorImplIntf *get_iterator(Graph::IndexType index_type, bool reverse);
+        Index::Index_IteratorImplIntf *get_iterator(Graph::IndexType index_type, const K &key,
+                                                    PropertyPredicate::Op op, bool reverse);
+        Index::Index_IteratorImplIntf *get_iterator(Graph::IndexType index_type, const K &min,
+                                                    const K &max, PropertyPredicate::Op op,
+                                                    bool reverse);
 
         // For statistics
         void index_stats_info(Graph::IndexStats &stats);

diff --git a/src/GraphImpl.h b/src/GraphImpl.h
@@ -44,9 +44,11 @@ namespace PMGD {
     struct RegionInfo;
 
     class GraphImpl {
+    public:
         typedef FixedAllocator NodeTable;
         typedef FixedAllocator EdgeTable;
 
+    private:
         struct GraphInfo;
 
         struct GraphInit {

diff --git a/src/Index.cc b/src/Index.cc
@@ -165,7 +165,9 @@ void Index::remove(const Property &p, void *n, GraphImpl *db)
     }
 }
 
-Index::Index_IteratorImplIntf *Index::get_iterator(const PropertyPredicate &pp, std::locale *loc, bool reverse)
+Index::Index_IteratorImplIntf *Index::get_iterator(Graph::IndexType index_type,
+                                        const PropertyPredicate &pp, std::locale *loc,
+                                        bool reverse)
 {
     const Property &p1 = pp.v1;
     const Property &p2 = pp.v2;
@@ -184,58 +186,58 @@ Index::Index_IteratorImplIntf *Index::get_iterator(const PropertyPredicate &pp,
             {
                 LongValueIndex *This = static_cast<LongValueIndex *>(this);
                 if (pp.op >= PropertyPredicate::GeLe)
-                    return This->get_iterator(p1.int_value(), p2.int_value(), pp.op, reverse);
+                    return This->get_iterator(index_type, p1.int_value(), p2.int_value(), pp.op, reverse);
                 else if (pp.op == PropertyPredicate::DontCare)
-                    return This->get_iterator(reverse);
+                    return This->get_iterator(index_type, reverse);
                 else
-                    return This->get_iterator(p1.int_value(), pp.op, reverse);
+                    return This->get_iterator(index_type, p1.int_value(), pp.op, reverse);
             }
             break;
         case PropertyType::Float:
             {
                 FloatValueIndex *This = static_cast<FloatValueIndex *>(this);
                 if (pp.op >= PropertyPredicate::GeLe)
-                    return This->get_iterator(p1.float_value(), p2.float_value(), pp.op, reverse);
+                    return This->get_iterator(index_type, p1.float_value(), p2.float_value(), pp.op, reverse);
                 else if (pp.op == PropertyPredicate::DontCare)
-                    return This->get_iterator(reverse);
+                    return This->get_iterator(index_type, reverse);
                 else
-                    return This->get_iterator(p1.float_value(), pp.op, reverse);
+                    return This->get_iterator(index_type, p1.float_value(), pp.op, reverse);
             }
             break;
         case PropertyType::Boolean:
             {
                 BoolValueIndex *This = static_cast<BoolValueIndex *>(this);
                 if (pp.op >= PropertyPredicate::GeLe)
-                    return This->get_iterator(p1.bool_value(), p2.bool_value(), pp.op, reverse);
+                    return This->get_iterator(index_type, p1.bool_value(), p2.bool_value(), pp.op, reverse);
                 else if (pp.op == PropertyPredicate::DontCare)
-                    return This->get_iterator(reverse);
+                    return This->get_iterator(index_type, reverse);
                 else
-                    return This->get_iterator(p1.bool_value(), pp.op, reverse);
+                    return This->get_iterator(index_type, p1.bool_value(), pp.op, reverse);
             }
             break;
         case PropertyType::Time:
             {
                 TimeValueIndex *This = static_cast<TimeValueIndex *>(this);
                 if (pp.op >= PropertyPredicate::GeLe)
-                    return This->get_iterator(p1.time_value(), p2.time_value(), pp.op, reverse);
+                    return This->get_iterator(index_type, p1.time_value(), p2.time_value(), pp.op, reverse);
                 else if (pp.op == PropertyPredicate::DontCare)
-                    return This->get_iterator(reverse);
+                    return This->get_iterator(index_type, reverse);
                 else
-                    return This->get_iterator(p1.time_value(), pp.op, reverse);
+                    return This->get_iterator(index_type, p1.time_value(), pp.op, reverse);
             }
             break;
         case PropertyType::String:
             {
                 StringValueIndex *This = static_cast<StringValueIndex *>(this);
                 if (pp.op == PropertyPredicate::DontCare)
-                    return This->get_iterator(reverse);
+                    return This->get_iterator(index_type, reverse);
                 TransientIndexString istr(p1.string_value(), *loc);
                 if (pp.op >= PropertyPredicate::GeLe) {
                     TransientIndexString istr2(p2.string_value(), *loc);
-                    return This->get_iterator(istr, istr2, pp.op, reverse);
+                    return This->get_iterator(index_type, istr, istr2, pp.op, reverse);
                 }
                 else
-                    return This->get_iterator(istr, pp.op, reverse);
+                    return This->get_iterator(index_type, istr, pp.op, reverse);
             }
             break;
         case PropertyType::NoValue:

diff --git a/src/Index.h b/src/Index.h
@@ -59,7 +59,9 @@ namespace PMGD {
 
         // Use a locale pointer here so that callers, where locale is
         // irrelevant, do not need to acquire it from the GraphImpl object.
-        Index_IteratorImplIntf *get_iterator(const PropertyPredicate &pp, std::locale *loc, bool reverse);
+        Index_IteratorImplIntf *get_iterator(Graph::IndexType index_type,
+                                             const PropertyPredicate &pp, std::locale *loc,
+                                             bool reverse);
 
         // Function to gather statistics
         Graph::IndexStats get_stats();

diff --git a/src/IndexManager.cc b/src/IndexManager.cc
@@ -397,7 +397,8 @@ Index::Index_IteratorImplIntf *IndexManager::get_iterator
     // This index can never be null cause we create it for each non-zero tag.
     // The second parameter here is the locale which we surely do not need for
     // a boolean property.
-    return prop0_idx->get_iterator(PropertyPredicate(0, PropertyPredicate::Eq, true), NULL, false);
+    return prop0_idx->get_iterator(index_type, PropertyPredicate(0, PropertyPredicate::Eq, true),
+                                    NULL, false);
 }
 
 void IndexManager::update

diff --git a/src/TransactionImpl.h b/src/TransactionImpl.h
@@ -47,7 +47,10 @@ namespace PMGD {
 
     class TransactionImpl {
         public:
-            enum LockTarget { NodeLock = 0, EdgeLock = 1, IndexLock = 2, NUM_LOCK_REGIONS = 3};
+            enum LockTarget { NodeLock = Graph::IndexType::NodeIndex,
+                              EdgeLock = Graph::IndexType::EdgeIndex,
+                              IndexLock,
+                              NUM_LOCK_REGIONS };
             enum LockState { LockNotFound = 0, ReadLock = 1, WriteLock = 2 };
 
         // For the iterator callbacks used by AvlTreeIndex and others, we need a
@@ -213,6 +216,13 @@ namespace PMGD {
             LockState acquire_lock(LockTarget which, const void *addr, bool write = false)
                 { return _locks[which].acquire_lock(addr, write); }
 
+            static void lock_node(const void *node, bool write)
+                { get_tx()->acquire_lock(NodeLock, node, write); }
+            static void lock_edge(const void *edge, bool write)
+                { get_tx()->acquire_lock(EdgeLock, edge, write); }
+            static void lock(Graph::IndexType type, const void *obj, bool write)
+                { get_tx()->acquire_lock(LockTarget(type), obj, write); }
+
             IteratorCallbacks &iterator_callbacks()
               { return _iter_callbacks; }
 

diff --git a/src/edge.cc b/src/edge.cc
@@ -50,23 +50,54 @@ EdgeID Edge::get_id() const
 
 EdgeID EdgeRef::get_id() const
 {
+    // get_id() for Edge takes care of locking.
     return edge()->get_id();
 }
 
+StringID Edge::get_tag() const
+{
+    return _tag;
+}
+
+Node &Edge::get_source() const
+{
+    TransactionImpl::lock_node(_src, false);
+    return *_src;
+}
+
+Node &Edge::get_destination() const
+{
+    TransactionImpl::lock_node(_dest, false);
+    return *_dest;
+}
+
 bool Edge::check_property(StringID id, Property &result) const
-    { return _property_list.check_property(id, result); }
+{
+    return _property_list.check_property(id, result);
+}
 
 Property Edge::get_property(StringID id) const
-    { return _property_list.get_property(id); }
+{
+    return _property_list.get_property(id);
+}
 
 PropertyIterator Edge::get_properties() const
-    { return _property_list.get_properties(); }
+{
+    return _property_list.get_properties();
+}
 
 void Edge::set_property(StringID id, const Property &new_value)
-    { _property_list.set_property(id, new_value, Graph::EdgeIndex, _tag, this); }
+{
+    TransactionImpl::lock_edge(this, true);
+    _property_list.set_property(id, new_value, Graph::EdgeIndex, _tag, this);
+}
 
 void Edge::remove_property(StringID id)
-    { _property_list.remove_property(id, Graph::EdgeIndex, _tag, this); }
+{
+    TransactionImpl::lock_edge(this, true);
+    _property_list.remove_property(id, Graph::EdgeIndex, _tag, this);
+}
 
+// Only called from Graph remove edge where edge is already locked.
 void Edge::remove_all_properties()
     { _property_list.remove_all_properties(Graph::EdgeIndex, _tag, this); }