pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390457541


##########
libs/utils/src/celix_hash_map.c:
##########
@@ -339,21 +327,27 @@ static bool celix_hashMap_remove(celix_hash_map_t* map, 
const char* strKey, long
         visit = visit->next;
     }
     if (removedEntry != NULL) {
+        char* removedKey = NULL;
+        if (map->keyType == CELIX_HASH_MAP_STRING_KEY) {
+            removedKey = (char*)removedEntry->key.strKey;
+        }
         celix_hashMap_destroyRemovedEntry(map, removedEntry);
+        if (removedKey) {
+            celix_hashMap_destroyRemovedKey(map, removedKey);
+        }
         return true;
     }
     return false;
 }
 
-static void celix_hashMap_init(
+celix_status_t celix_hashMap_init(
         celix_hash_map_t* map,
         celix_hash_map_key_type_e keyType,
         unsigned int initialCapacity,
         double loadFactor,
         unsigned int (*hashKeyFn)(const celix_hash_map_key_t*),
         bool (*equalsKeyFn)(const celix_hash_map_key_t*, const 
celix_hash_map_key_t*)) {
     map->loadFactor = loadFactor;

Review Comment:
   Interesting. The load factor of 0.75 was and still is used in the deprecated 
hash map. I did reading up on hash maps when I created the long/string hash 
map, but apparently missed this and reused the 0.75 factor. 
   
   I spent some hours tweaking the max load factor and hash functions combined 
with the already existing hash map benchmarks. Based on that result I changed 
the default long hash, refactored some code to prevent some function calls and 
changed the max load factor. 
   
   Max load factor is now 5.0 and the long hash function is also changed (uses 
a prime number for an overall better distribution in the hash map).
   
   I quite certain more improvements are possible for the long hash map and 
this can also been seen in the benchmark reference (`std::unordered_map with a 
long key`). 
   The celix string hash map performance, for finding an entry in the hash map, 
is quite close to the std::unordered_map with a string key. 
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to