improve nodestore; don't stop searching for nodes at a tombstone value

src/server/ua_nodestore.c

@@ -10,6 +10,8 @@ typedef struct UA_NodeStoreEntry {
     UA_Node node;
 } UA_NodeStoreEntry;
 
+#define UA_NODESTORE_TOMBSTONE ((UA_NodeStoreEntry*)0x01)
+
 struct UA_NodeStore {
     UA_NodeStoreEntry **entries;
     UA_UInt32 size;
@@ -20,7 +22,7 @@ struct UA_NodeStore {
 #include "ua_nodestore_hash.inc"
 
 /* The size of the hash-map is always a prime number. They are chosen to be
-   close to the next power of 2. So the size ca. doubles with each prime. */
+ * close to the next power of 2. So the size ca. doubles with each prime. */
 static hash_t const primes[] = {
     7,         13,         31,         61,         127,         251,
     509,       1021,       2039,       4093,       8191,        16381,
@@ -29,7 +31,8 @@ static hash_t const primes[] = {
     134217689, 268435399,  536870909,  1073741789, 2147483647,  4294967291
 };
 
-static UA_UInt16 higher_prime_index(hash_t n) {
+static UA_UInt16
+higher_prime_index(hash_t n) {
     UA_UInt16 low  = 0;
     UA_UInt16 high = (UA_UInt16)(sizeof(primes) / sizeof(hash_t));
     while(low != high) {
@@ -42,7 +45,8 @@ static UA_UInt16 higher_prime_index(hash_t n) {
     return low;
 }
 
-static UA_NodeStoreEntry * instantiateEntry(UA_NodeClass nodeClass) {
+static UA_NodeStoreEntry *
+instantiateEntry(UA_NodeClass nodeClass) {
     size_t size = sizeof(UA_NodeStoreEntry) - sizeof(UA_Node);
     switch(nodeClass) {
     case UA_NODECLASS_OBJECT:
@@ -79,63 +83,75 @@ static UA_NodeStoreEntry * instantiateEntry(UA_NodeClass nodeClass) {
     return entry;
 }
 
-static void deleteEntry(UA_NodeStoreEntry *entry) {
+static void
+deleteEntry(UA_NodeStoreEntry *entry) {
     UA_Node_deleteMembersAnyNodeClass(&entry->node);
     UA_free(entry);
 }
 
-/* Returns true if an entry was found under the nodeid. Otherwise, returns
-   false and sets slot to a pointer to the next free slot. */
-static UA_Boolean
-containsNodeId(const UA_NodeStore *ns, const UA_NodeId *nodeid, UA_NodeStoreEntry ***entry) {
+/* returns slot of a valid node or null */
+static UA_NodeStoreEntry **
+findNode(const UA_NodeStore *ns, const UA_NodeId *nodeid) {
     hash_t h = hash(nodeid);
     UA_UInt32 size = ns->size;
     hash_t idx = mod(h, size);
-    UA_NodeStoreEntry *e = ns->entries[idx];
+    hash_t hash2 = mod2(h, size);
 
-    if(!e) {
-        *entry = &ns->entries[idx];
-        return false;
+    while(true) {
+        UA_NodeStoreEntry *e = ns->entries[idx];
+        if(!e)
+            return NULL;
+        if(e > UA_NODESTORE_TOMBSTONE &&
+           UA_NodeId_equal(&e->node.nodeId, nodeid))
+            return &ns->entries[idx];
+        idx += hash2;
+        if(idx >= size)
+            idx -= size;
     }
 
-    if(UA_NodeId_equal(&e->node.nodeId, nodeid)) {
-        *entry = &ns->entries[idx];
-        return true;
-    }
+    /* NOTREACHED */
+    return NULL;
+}
 
+/* returns an empty slot or null if the nodeid exists */
+static UA_NodeStoreEntry **
+findSlot(const UA_NodeStore *ns, const UA_NodeId *nodeid) {
+    hash_t h = hash(nodeid);
+    UA_UInt32 size = ns->size;
+    hash_t idx = mod(h, size);
     hash_t hash2 = mod2(h, size);
-    for(;;) {
+
+    while(true) {
+        UA_NodeStoreEntry *e = ns->entries[idx];
+        if(e > UA_NODESTORE_TOMBSTONE &&
+           UA_NodeId_equal(&e->node.nodeId, nodeid))
+            return NULL;
+        if(ns->entries[idx] <= UA_NODESTORE_TOMBSTONE)
+            return &ns->entries[idx];
         idx += hash2;
         if(idx >= size)
             idx -= size;
-        e = ns->entries[idx];
-        if(!e) {
-            *entry = &ns->entries[idx];
-            return false;
-        }
-        if(UA_NodeId_equal(&e->node.nodeId, nodeid)) {
-            *entry = &ns->entries[idx];
-            return true;
-        }
     }
 
     /* NOTREACHED */
-    return true;
+    return NULL;
 }
 
 /* The occupancy of the table after the call will be about 50% */
-static UA_StatusCode expand(UA_NodeStore *ns) {
+static UA_StatusCode
+expand(UA_NodeStore *ns) {
     UA_UInt32 osize = ns->size;
     UA_UInt32 count = ns->count;
-    /* Resize only when table after removal of unused elements is either too full or too empty  */
+    /* Resize only when table after removal of unused elements is either too
+       full or too empty */
     if(count * 2 < osize && (count * 8 > osize || osize <= UA_NODESTORE_MINSIZE))
         return UA_STATUSCODE_GOOD;
 
     UA_NodeStoreEntry **oentries = ns->entries;
     UA_UInt32 nindex = higher_prime_index(count * 2);
     UA_UInt32 nsize = primes[nindex];
-    UA_NodeStoreEntry **nentries;
-    if(!(nentries = UA_calloc(nsize, sizeof(UA_NodeStoreEntry*))))
+    UA_NodeStoreEntry **nentries = UA_calloc(nsize, sizeof(UA_NodeStoreEntry*));
+    if(!nentries)
         return UA_STATUSCODE_BADOUTOFMEMORY;
 
     ns->entries = nentries;
@@ -146,8 +162,7 @@ static UA_StatusCode expand(UA_NodeStore *ns) {
     for(size_t i = 0, j = 0; i < osize && j < count; i++) {
         if(!oentries[i])
             continue;
-        UA_NodeStoreEntry **e;
-        containsNodeId(ns, &oentries[i]->node.nodeId, &e);  /* We know this returns an empty entry here */
+        UA_NodeStoreEntry **e = findSlot(ns, &oentries[i]->node.nodeId);
         *e = oentries[i];
         j++;
     }
@@ -160,21 +175,24 @@ static UA_StatusCode expand(UA_NodeStore *ns) {
 /* Exported functions */
 /**********************/
 
-UA_NodeStore * UA_NodeStore_new(void) {
-    UA_NodeStore *ns;
-    if(!(ns = UA_malloc(sizeof(UA_NodeStore))))
+UA_NodeStore *
+UA_NodeStore_new(void) {
+    UA_NodeStore *ns = ns = UA_malloc(sizeof(UA_NodeStore));
+    if(!ns)
         return NULL;
     ns->sizePrimeIndex = higher_prime_index(UA_NODESTORE_MINSIZE);
     ns->size = primes[ns->sizePrimeIndex];
     ns->count = 0;
-    if(!(ns->entries = UA_calloc(ns->size, sizeof(UA_NodeStoreEntry*)))) {
+    ns->entries = UA_calloc(ns->size, sizeof(UA_NodeStoreEntry*));
+    if(!ns->entries) {
         UA_free(ns);
         return NULL;
     }
     return ns;
 }
 
-void UA_NodeStore_delete(UA_NodeStore *ns) {
+void
+UA_NodeStore_delete(UA_NodeStore *ns) {
     UA_UInt32 size = ns->size;
     UA_NodeStoreEntry **entries = ns->entries;
     for(UA_UInt32 i = 0; i < size; i++) {
@@ -185,18 +203,21 @@ void UA_NodeStore_delete(UA_NodeStore *ns) {
     UA_free(ns);
 }
 
-UA_Node * UA_NodeStore_newNode(UA_NodeClass class) {
+UA_Node *
+UA_NodeStore_newNode(UA_NodeClass class) {
     UA_NodeStoreEntry *entry = instantiateEntry(class);
     if(!entry)
         return NULL;
     return (UA_Node*)&entry->node;
 }
 
-void UA_NodeStore_deleteNode(UA_Node *node) {
+void
+UA_NodeStore_deleteNode(UA_Node *node) {
     deleteEntry(container_of(node, UA_NodeStoreEntry, node));
 }
 
-UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node) {
+UA_StatusCode
+UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node) {
     if(ns->size * 3 <= ns->count * 4) {
         if(expand(ns) != UA_STATUSCODE_GOOD)
             return UA_STATUSCODE_BADINTERNALERROR;
@@ -207,22 +228,24 @@ UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node) {
     tempNodeid.namespaceIndex = 0;
     UA_NodeStoreEntry **entry;
     if(UA_NodeId_isNull(&tempNodeid)) {
+        /* create a random nodeid */
         if(node->nodeId.namespaceIndex == 0)
             node->nodeId.namespaceIndex = 1;
-        /* find a free nodeid */
         UA_UInt32 identifier = ns->count+1; // start value
         UA_UInt32 size = ns->size;
         hash_t increase = mod2(identifier, size);
         while(true) {
             node->nodeId.identifier.numeric = identifier;
-            if(!containsNodeId(ns, &node->nodeId, &entry))
+            entry = findSlot(ns, &node->nodeId);
+            if(entry)
                 break;
             identifier += increase;
             if(identifier >= size)
                 identifier -= size;
         }
     } else {
-        if(containsNodeId(ns, &node->nodeId, &entry)) {
+        entry = findSlot(ns, &node->nodeId);
+        if(!entry) {
             deleteEntry(container_of(node, UA_NodeStoreEntry, node));
             return UA_STATUSCODE_BADNODEIDEXISTS;
         }
@@ -235,29 +258,32 @@ UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node) {
 
 UA_StatusCode
 UA_NodeStore_replace(UA_NodeStore *ns, UA_Node *node) {
-    UA_NodeStoreEntry **entry;
-    if(!containsNodeId(ns, &node->nodeId, &entry))
+    UA_NodeStoreEntry **entry = findNode(ns, &node->nodeId);
+    if(!entry)
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
     UA_NodeStoreEntry *newEntry = container_of(node, UA_NodeStoreEntry, node);
     if(*entry != newEntry->orig) {
+        // the node was replaced since the copy was made
         deleteEntry(newEntry);
-        return UA_STATUSCODE_BADINTERNALERROR; // the node was replaced since the copy was made
+        return UA_STATUSCODE_BADINTERNALERROR;
     }
     deleteEntry(*entry);
     *entry = newEntry;
     return UA_STATUSCODE_GOOD;
 }
 
-const UA_Node * UA_NodeStore_get(UA_NodeStore *ns, const UA_NodeId *nodeid) {
-    UA_NodeStoreEntry **entry;
-    if(!containsNodeId(ns, nodeid, &entry))
+const UA_Node *
+UA_NodeStore_get(UA_NodeStore *ns, const UA_NodeId *nodeid) {
+    UA_NodeStoreEntry **entry = findNode(ns, nodeid);
+    if(!entry)
         return NULL;
     return (const UA_Node*)&(*entry)->node;
 }
 
-UA_Node * UA_NodeStore_getCopy(UA_NodeStore *ns, const UA_NodeId *nodeid) {
-    UA_NodeStoreEntry **slot;
-    if(!containsNodeId(ns, nodeid, &slot))
+UA_Node *
+UA_NodeStore_getCopy(UA_NodeStore *ns, const UA_NodeId *nodeid) {
+    UA_NodeStoreEntry **slot = findNode(ns, nodeid);
+    if(!slot)
         return NULL;
     UA_NodeStoreEntry *entry = *slot;
     UA_NodeStoreEntry *new = instantiateEntry(entry->node.nodeClass);
@@ -267,16 +293,17 @@ UA_Node * UA_NodeStore_getCopy(UA_NodeStore *ns, const UA_NodeId *nodeid) {
         deleteEntry(new);
         return NULL;
     }
-    new->orig = entry;
+    new->orig = entry; // store the pointer to the original
     return &new->node;
 }
 
-UA_StatusCode UA_NodeStore_remove(UA_NodeStore *ns, const UA_NodeId *nodeid) {
-    UA_NodeStoreEntry **slot;
-    if(!containsNodeId(ns, nodeid, &slot))
+UA_StatusCode
+UA_NodeStore_remove(UA_NodeStore *ns, const UA_NodeId *nodeid) {
+    UA_NodeStoreEntry **slot = findNode(ns, nodeid);
+    if(!slot)
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
     deleteEntry(*slot);
-    *slot = NULL;
+    *slot = UA_NODESTORE_TOMBSTONE;
     ns->count--;
     /* Downsize the hashmap if it is very empty */
     if(ns->count * 8 < ns->size && ns->size > 32)
@@ -284,9 +311,10 @@ UA_StatusCode UA_NodeStore_remove(UA_NodeStore *ns, const UA_NodeId *nodeid) {
     return UA_STATUSCODE_GOOD;
 }
 
-void UA_NodeStore_iterate(UA_NodeStore *ns, UA_NodeStore_nodeVisitor visitor) {
+void
+UA_NodeStore_iterate(UA_NodeStore *ns, UA_NodeStore_nodeVisitor visitor) {
     for(UA_UInt32 i = 0; i < ns->size; i++) {
-        if(ns->entries[i])
+        if(ns->entries[i] > UA_NODESTORE_TOMBSTONE)
             visitor((UA_Node*)&ns->entries[i]->node);
     }
 }