refactor(plugin): Nodestore: Reduce the cache pressure from the hash

Put the hashes in the slot-array. This saves an expensive lookup from
main memory for the nodes that are seldomly visited.

plugins/ua_nodestore_hashmap.c

@@ -24,7 +24,6 @@
  * nodes. */
 
 typedef struct UA_NodeMapEntry {
-    UA_UInt32 nodeIdHash;
     struct UA_NodeMapEntry *orig; /* the version this is a copy from (or NULL) */
     UA_UInt16 refCount; /* How many consumers have a reference to the node? */
     UA_Boolean deleted; /* Node was marked as deleted and can be deleted when refCount == 0 */
@@ -35,7 +34,12 @@ typedef struct UA_NodeMapEntry {
 #define UA_NODEMAP_TOMBSTONE ((UA_NodeMapEntry*)0x01)
 
 typedef struct {
-    UA_NodeMapEntry **entries;
+    UA_NodeMapEntry *entry;
+    UA_UInt32 nodeIdHash;
+} UA_NodeMapSlot;
+
+typedef struct {
+    UA_NodeMapSlot *slots;
     UA_UInt32 size;
     UA_UInt32 count;
     UA_UInt32 sizePrimeIndex;
@@ -73,7 +77,7 @@ higher_prime_index(UA_UInt32 n) {
 }
 
 /* Returns an empty slot or null if the nodeid exists or if no empty slot is found. */
-static UA_NodeMapEntry **
+static UA_NodeMapSlot *
 findFreeSlot(const UA_NodeMap *ns, const UA_NodeId *nodeid) {
     UA_UInt32 h = UA_NodeId_hash(nodeid);
     UA_UInt32 size = ns->size;
@@ -81,21 +85,21 @@ findFreeSlot(const UA_NodeMap *ns, const UA_NodeId *nodeid) {
     UA_UInt32 startIdx = (UA_UInt32)idx;
     UA_UInt32 hash2 = mod2(h, size);
 
-    UA_NodeMapEntry **candidate = NULL;
+    UA_NodeMapSlot *candidate = NULL;
     do {
-        UA_NodeMapEntry *entry = ns->entries[(UA_UInt32)idx];
+        UA_NodeMapSlot *slot = &ns->slots[(UA_UInt32)idx];
 
-        if(entry > UA_NODEMAP_TOMBSTONE) {
+        if(slot->entry > UA_NODEMAP_TOMBSTONE) {
             /* A Node with the NodeId does already exist */
-            if(entry->nodeIdHash == h &&
-               UA_NodeId_equal(&entry->node.nodeId, nodeid))
+            if(slot->nodeIdHash == h &&
+               UA_NodeId_equal(&slot->entry->node.nodeId, nodeid))
                 return NULL;
         } else {
             /* Found a candidate node */
             if(!candidate)
-                candidate = &ns->entries[(UA_UInt32)idx];
+                candidate = slot;
             /* No matching node can come afterwards */
-            if(entry == NULL)
+            if(slot->entry == NULL)
                 return candidate;
         }
 
@@ -117,28 +121,28 @@ expand(UA_NodeMap *ns) {
     if(count * 2 < osize && (count * 8 > osize || osize <= UA_NODEMAP_MINSIZE))
         return UA_STATUSCODE_GOOD;
 
-    UA_NodeMapEntry **oentries = ns->entries;
+    UA_NodeMapSlot *oslots = ns->slots;
     UA_UInt32 nindex = higher_prime_index(count * 2);
     UA_UInt32 nsize = primes[nindex];
-    UA_NodeMapEntry **nentries = (UA_NodeMapEntry **)UA_calloc(nsize, sizeof(UA_NodeMapEntry*));
-    if(!nentries)
+    UA_NodeMapSlot *nslots= (UA_NodeMapSlot*)UA_calloc(nsize, sizeof(UA_NodeMapSlot));
+    if(!nslots)
         return UA_STATUSCODE_BADOUTOFMEMORY;
 
-    ns->entries = nentries;
+    ns->slots = nslots;
     ns->size = nsize;
     ns->sizePrimeIndex = nindex;
 
     /* recompute the position of every entry and insert the pointer */
     for(size_t i = 0, j = 0; i < osize && j < count; ++i) {
-        if(oentries[i] <= UA_NODEMAP_TOMBSTONE)
+        if(oslots[i].entry <= UA_NODEMAP_TOMBSTONE)
             continue;
-        UA_NodeMapEntry **e = findFreeSlot(ns, &oentries[i]->node.nodeId);
-        UA_assert(e);
-        *e = oentries[i];
+        UA_NodeMapSlot *s = findFreeSlot(ns, &oslots[i].entry->node.nodeId);
+        UA_assert(s);
+        *s = oslots[i];
         ++j;
     }
 
-    UA_free(oentries);
+    UA_free(oslots);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -193,9 +197,9 @@ cleanupNodeMapEntry(UA_NodeMapEntry *entry) {
 }
 
 static UA_StatusCode
-clearSlot(UA_NodeMap *ns, UA_NodeMapEntry **slot) {
-    UA_NodeMapEntry *entry = *slot;
-    if(UA_atomic_cmpxchg((void**)slot, entry, UA_NODEMAP_TOMBSTONE) != entry)
+clearSlot(UA_NodeMap *ns, UA_NodeMapSlot *slot) {
+    UA_NodeMapEntry *entry = slot->entry;
+    if(UA_atomic_cmpxchg((void**)&slot->entry, entry, UA_NODEMAP_TOMBSTONE) != entry)
         return UA_STATUSCODE_BADINTERNALERROR;
     entry->deleted = true;
     cleanupNodeMapEntry(entry);
@@ -206,7 +210,7 @@ clearSlot(UA_NodeMap *ns, UA_NodeMapEntry **slot) {
     return UA_STATUSCODE_GOOD;
 }
 
-static UA_NodeMapEntry **
+static UA_NodeMapSlot *
 findOccupiedSlot(const UA_NodeMap *ns, const UA_NodeId *nodeid) {
     UA_UInt32 h = UA_NodeId_hash(nodeid);
     UA_UInt32 size = ns->size;
@@ -215,16 +219,16 @@ findOccupiedSlot(const UA_NodeMap *ns, const UA_NodeId *nodeid) {
     UA_UInt32 startIdx = (UA_UInt32)idx;
 
     do {
-        UA_NodeMapEntry *entry = ns->entries[(UA_UInt32)idx];
+        UA_NodeMapSlot *slot= &ns->slots[(UA_UInt32)idx];
 
         /* Found a candidate */
-        if(entry > UA_NODEMAP_TOMBSTONE) {
-            if(entry->nodeIdHash == h &&
-               UA_NodeId_equal(&entry->node.nodeId, nodeid))
-                return &ns->entries[(UA_UInt32)idx];
+        if(slot->entry > UA_NODEMAP_TOMBSTONE) {
+            if(slot->nodeIdHash == h &&
+               UA_NodeId_equal(&slot->entry->node.nodeId, nodeid))
+                return slot;
         } else {
             /* No entry can be found afterwards */
-            if(entry == NULL)
+            if(slot->entry == NULL)
                 return NULL;
         }
 
@@ -258,11 +262,11 @@ UA_NodeMap_deleteNode(void *context, UA_Node *node) {
 static const UA_Node *
 UA_NodeMap_getNode(void *context, const UA_NodeId *nodeid) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-    UA_NodeMapEntry **entry = findOccupiedSlot(ns, nodeid);
-    if(!entry)
+    UA_NodeMapSlot *slot = findOccupiedSlot(ns, nodeid);
+    if(!slot)
         return NULL;
-    ++(*entry)->refCount;
-    return (const UA_Node*)&(*entry)->node;
+    ++slot->entry->refCount;
+    return &slot->entry->node;
 }
 
 static void
@@ -280,10 +284,10 @@ static UA_StatusCode
 UA_NodeMap_getNodeCopy(void *context, const UA_NodeId *nodeid,
                        UA_Node **outNode) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-    UA_NodeMapEntry **slot = findOccupiedSlot(ns, nodeid);
+    UA_NodeMapSlot *slot = findOccupiedSlot(ns, nodeid);
     if(!slot)
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
-    UA_NodeMapEntry *entry = *slot;
+    UA_NodeMapEntry *entry = slot->entry;
     UA_NodeMapEntry *newItem = createEntry(entry->node.nodeClass);
     if(!newItem)
         return UA_STATUSCODE_BADOUTOFMEMORY;
@@ -300,7 +304,7 @@ UA_NodeMap_getNodeCopy(void *context, const UA_NodeId *nodeid,
 static UA_StatusCode
 UA_NodeMap_removeNode(void *context, const UA_NodeId *nodeid) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-    UA_NodeMapEntry **slot = findOccupiedSlot(ns, nodeid);
+    UA_NodeMapSlot *slot = findOccupiedSlot(ns, nodeid);
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
     if(slot)
         retval = clearSlot(ns, slot);
@@ -318,7 +322,7 @@ UA_NodeMap_insertNode(void *context, UA_Node *node,
             return UA_STATUSCODE_BADINTERNALERROR;
     }
 
-    UA_NodeMapEntry **slot;
+    UA_NodeMapSlot *slot;
     if(node->nodeId.identifierType == UA_NODEIDTYPE_NUMERIC &&
        node->nodeId.identifier.numeric == 0) {
         /* Create a random nodeid: Start at least with 50,000 to make sure we
@@ -365,11 +369,11 @@ UA_NodeMap_insertNode(void *context, UA_Node *node,
 
     /* Store the hash */
     UA_NodeMapEntry *newEntry = container_of(node, UA_NodeMapEntry, node);
-    newEntry->nodeIdHash = UA_NodeId_hash(&node->nodeId);
+    slot->nodeIdHash = UA_NodeId_hash(&node->nodeId);
 
     /* Insert the node */
-    UA_NodeMapEntry *oldEntry = *slot;
-    if(UA_atomic_cmpxchg((void**)slot, oldEntry, newEntry) != oldEntry) {
+    UA_NodeMapEntry *oldEntry = slot->entry;
+    if(UA_atomic_cmpxchg((void**)&slot->entry, oldEntry, newEntry) != oldEntry) {
         deleteNodeMapEntry(container_of(node, UA_NodeMapEntry, node));
         return UA_STATUSCODE_BADNODEIDEXISTS;
     }
@@ -384,12 +388,12 @@ UA_NodeMap_replaceNode(void *context, UA_Node *node) {
     UA_NodeMapEntry *newEntry = container_of(node, UA_NodeMapEntry, node);
 
     /* Find the node */
-    UA_NodeMapEntry **slot = findOccupiedSlot(ns, &node->nodeId);
+    UA_NodeMapSlot *slot = findOccupiedSlot(ns, &node->nodeId);
     if(!slot) {
         deleteNodeMapEntry(newEntry);
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
     }
-    UA_NodeMapEntry *oldEntry = *slot;
+    UA_NodeMapEntry *oldEntry = slot->entry;
 
     /* The node was already updated since the copy was made? */
     if(oldEntry != newEntry->orig) {
@@ -397,11 +401,8 @@ UA_NodeMap_replaceNode(void *context, UA_Node *node) {
         return UA_STATUSCODE_BADINTERNALERROR;
     }
 
-    /* Store the hash */
-    newEntry->nodeIdHash = oldEntry->nodeIdHash;
-
     /* Replace the entry with an atomic operation */
-    if(UA_atomic_cmpxchg((void**)slot, oldEntry, newEntry) != oldEntry) {
+    if(UA_atomic_cmpxchg((void**)&slot->entry, oldEntry, newEntry) != oldEntry) {
         deleteNodeMapEntry(newEntry);
         return UA_STATUSCODE_BADINTERNALERROR;
     }
@@ -416,12 +417,13 @@ UA_NodeMap_iterate(void *context, UA_NodestoreVisitor visitor,
                    void *visitorContext) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
     for(UA_UInt32 i = 0; i < ns->size; ++i) {
-        if(ns->entries[i] > UA_NODEMAP_TOMBSTONE) {
-            UA_NodeMapEntry *entry = ns->entries[i];
-            entry->refCount++;
-            visitor(visitorContext, &entry->node);
-            entry->refCount--;
-            cleanupNodeMapEntry(entry);
+        UA_NodeMapSlot *slot = &ns->slots[i];
+        if(slot->entry > UA_NODEMAP_TOMBSTONE) {
+            /* The visitor can delete the node. So refcount here. */
+            slot->entry->refCount++;
+            visitor(visitorContext, &slot->entry->node);
+            slot->entry->refCount--;
+            cleanupNodeMapEntry(slot->entry);
         }
     }
 }
@@ -430,16 +432,16 @@ static void
 UA_NodeMap_delete(void *context) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
     UA_UInt32 size = ns->size;
-    UA_NodeMapEntry **entries = ns->entries;
+    UA_NodeMapSlot *slots = ns->slots;
     for(UA_UInt32 i = 0; i < size; ++i) {
-        if(entries[i] > UA_NODEMAP_TOMBSTONE) {
+        if(slots[i].entry > UA_NODEMAP_TOMBSTONE) {
             /* On debugging builds, check that all nodes were release */
-            UA_assert(entries[i]->refCount == 0);
+            UA_assert(slots[i].entry->refCount == 0);
             /* Delete the node */
-            deleteNodeMapEntry(entries[i]);
+            deleteNodeMapEntry(slots[i].entry);
         }
     }
-    UA_free(ns->entries);
+    UA_free(ns->slots);
     UA_free(ns);
 }
 
@@ -452,9 +454,9 @@ UA_Nodestore_HashMap(UA_Nodestore *ns) {
     nodemap->sizePrimeIndex = higher_prime_index(UA_NODEMAP_MINSIZE);
     nodemap->size = primes[nodemap->sizePrimeIndex];
     nodemap->count = 0;
-    nodemap->entries = (UA_NodeMapEntry**)
-        UA_calloc(nodemap->size, sizeof(UA_NodeMapEntry*));
-    if(!nodemap->entries) {
+    nodemap->slots = (UA_NodeMapSlot*)
+        UA_calloc(nodemap->size, sizeof(UA_NodeMapSlot));
+    if(!nodemap->slots) {
         UA_free(nodemap);
         return UA_STATUSCODE_BADOUTOFMEMORY;
     }