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


##########
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:
   Here are the hash function benchmark results (on my machine, using a release 
build type and cpupower on performance):
   
   hash1 is the long hash function used in celix_long_hash_map at the start of 
this PR
   hash2 is the long hash function used in the deprecated hashmap 
   hash3 is a altered long hash function which uses a prime.
   
   I also had a short look into 
[murmur3](https://github.com/logrhythm/MurMurHash/blob/master/src/MurMurHash.cpp)
 and [FNV](https://github.com/fnvhash/libfnv/tree/master) hash, but for the 
moment I think this requires too much work (technical and license wise)
   
   # Hash1, max load 5
   ```
   
-----------------------------------------------------------------------------------------------------------------------------------------
   Benchmark                                                                    
           Time             CPU   Iterations UserCounters...
   
-----------------------------------------------------------------------------------------------------------------------------------------
   LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time           
       0.432 ns        0.432 ns   1000000000 items_per_second=2.31246G/s
   LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time          
       0.433 ns        0.433 ns   1000000000 items_per_second=2.31041G/s
   LongHashmapBenchmark_findEntryFromStdMap/1000/process_time/real_time         
       0.433 ns        0.433 ns   1000000000 items_per_second=2.31151G/s
   LongHashmapBenchmark_findEntryFromStdMap/10000/process_time/real_time        
       0.434 ns        0.434 ns   1000000000 items_per_second=2.30551G/s
   LongHashmapBenchmark_findEntryFromStdMap/100000/process_time/real_time       
       0.433 ns        0.433 ns   1000000000 items_per_second=2.31037G/s
   LongHashmapBenchmark_findEntryFromStdMap/1000000/process_time/real_time      
       0.433 ns        0.432 ns   1000000000 items_per_second=2.30713G/s
   LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time         
        2.38 ns         2.38 ns    293482551 averageNrOfEntriesPerBucket=0.625 
items_per_second=420.077M/s nrOfBuckets=16 resizeCount=0 
stdDeviationNrOfEntriesPerBucket=0.856957
   LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time        
        2.64 ns         2.63 ns    268351618 averageNrOfEntriesPerBucket=3.125 
items_per_second=379.474M/s nrOfBuckets=32 resizeCount=1 
stdDeviationNrOfEntriesPerBucket=1.91621
   LongHashmapBenchmark_findEntryFromCelixMap/1000/process_time/real_time       
        2.16 ns         2.16 ns    323130939 
averageNrOfEntriesPerBucket=3.90625 items_per_second=462.94M/s nrOfBuckets=256 
resizeCount=4 stdDeviationNrOfEntriesPerBucket=1.80467
   LongHashmapBenchmark_findEntryFromCelixMap/10000/process_time/real_time      
        3.90 ns         3.89 ns    183397882 
averageNrOfEntriesPerBucket=4.88281 items_per_second=256.477M/s 
nrOfBuckets=2.048k resizeCount=7 stdDeviationNrOfEntriesPerBucket=2.20062
   LongHashmapBenchmark_findEntryFromCelixMap/100000/process_time/real_time     
        2.38 ns         2.38 ns    294279918 
averageNrOfEntriesPerBucket=3.75601 items_per_second=420.113M/s 
nrOfBuckets=26.624k resizeCount=11 stdDeviationNrOfEntriesPerBucket=1.93968
   LongHashmapBenchmark_findEntryFromCelixMap/1000000/process_time/real_time    
        2.62 ns         2.62 ns    270684694 
averageNrOfEntriesPerBucket=4.98246 items_per_second=380.976M/s 
nrOfBuckets=200.704k resizeCount=28 stdDeviationNrOfEntriesPerBucket=2.23419
   LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time    
        3.67 ns         3.67 ns    190401851 items_per_second=272.19M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/100/process_time/real_time   
        3.69 ns         3.68 ns    190481474 items_per_second=271.21M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/1000/process_time/real_time  
        3.69 ns         3.68 ns    189597715 items_per_second=271.084M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/10000/process_time/real_time 
        3.69 ns         3.68 ns    189618122 items_per_second=271.035M/s
   
LongHashmapBenchmark_findEntryFromDeprecatedMap/100000/process_time/real_time   
     3.69 ns         3.68 ns    189681903 items_per_second=271.169M/s
   
LongHashmapBenchmark_findEntryFromDeprecatedMap/1000000/process_time/real_time  
     3.67 ns         3.67 ns    190523011 items_per_second=272.179M/s
   ```
   
   
   # Hash 2, max load 5
   ```
   
-----------------------------------------------------------------------------------------------------------------------------------------
   Benchmark                                                                    
           Time             CPU   Iterations UserCounters...
   
-----------------------------------------------------------------------------------------------------------------------------------------
   LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time           
       0.428 ns        0.428 ns   1000000000 items_per_second=2.33786G/s
   LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time          
       0.427 ns        0.427 ns   1000000000 items_per_second=2.34001G/s
   LongHashmapBenchmark_findEntryFromStdMap/1000/process_time/real_time         
       0.428 ns        0.428 ns   1000000000 items_per_second=2.33579G/s
   LongHashmapBenchmark_findEntryFromStdMap/10000/process_time/real_time        
       0.427 ns        0.427 ns   1000000000 items_per_second=2.3403G/s
   LongHashmapBenchmark_findEntryFromStdMap/100000/process_time/real_time       
       0.427 ns        0.427 ns   1000000000 items_per_second=2.34037G/s
   LongHashmapBenchmark_findEntryFromStdMap/1000000/process_time/real_time      
       0.428 ns        0.428 ns   1000000000 items_per_second=2.33835G/s
   LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time         
        2.56 ns         2.56 ns    272918800 averageNrOfEntriesPerBucket=0.625 
items_per_second=390.34M/s nrOfBuckets=16 resizeCount=0 
stdDeviationNrOfEntriesPerBucket=0.695971
   LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time        
        3.76 ns         3.76 ns    186188393 averageNrOfEntriesPerBucket=3.125 
items_per_second=266.203M/s nrOfBuckets=32 resizeCount=1 
stdDeviationNrOfEntriesPerBucket=1.93245
   LongHashmapBenchmark_findEntryFromCelixMap/1000/process_time/real_time       
        2.78 ns         2.78 ns    251731622 
averageNrOfEntriesPerBucket=3.90625 items_per_second=359.86M/s nrOfBuckets=256 
resizeCount=4 stdDeviationNrOfEntriesPerBucket=2.00366
   LongHashmapBenchmark_findEntryFromCelixMap/10000/process_time/real_time      
        2.99 ns         2.99 ns    233585592 
averageNrOfEntriesPerBucket=4.88281 items_per_second=333.894M/s 
nrOfBuckets=2.048k resizeCount=7 stdDeviationNrOfEntriesPerBucket=2.14352
   LongHashmapBenchmark_findEntryFromCelixMap/100000/process_time/real_time     
        3.00 ns         3.00 ns    233887362 
averageNrOfEntriesPerBucket=3.75601 items_per_second=333.838M/s 
nrOfBuckets=26.624k resizeCount=11 stdDeviationNrOfEntriesPerBucket=1.94674
   LongHashmapBenchmark_findEntryFromCelixMap/1000000/process_time/real_time    
        2.99 ns         2.99 ns    234075293 
averageNrOfEntriesPerBucket=4.98246 items_per_second=334.062M/s 
nrOfBuckets=200.704k resizeCount=28 stdDeviationNrOfEntriesPerBucket=2.22839
   LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time    
        3.64 ns         3.64 ns    192084107 items_per_second=274.697M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/100/process_time/real_time   
        3.64 ns         3.64 ns    192187264 items_per_second=274.762M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/1000/process_time/real_time  
        3.64 ns         3.64 ns    192202353 items_per_second=274.977M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/10000/process_time/real_time 
        3.63 ns         3.63 ns    192491569 items_per_second=275.124M/s
   
LongHashmapBenchmark_findEntryFromDeprecatedMap/100000/process_time/real_time   
     3.63 ns         3.63 ns    192638166 items_per_second=275.242M/s
   
LongHashmapBenchmark_findEntryFromDeprecatedMap/1000000/process_time/real_time  
     3.63 ns         3.63 ns    193028763 items_per_second=275.699M/s
   ```
   
   # Hash3, max load 5
   ```
   
-----------------------------------------------------------------------------------------------------------------------------------------
   Benchmark                                                                    
           Time             CPU   Iterations UserCounters...
   
-----------------------------------------------------------------------------------------------------------------------------------------
   LongHashmapBenchmark_findEntryFromStdMap/10/process_time/real_time           
       0.414 ns        0.414 ns   1000000000 items_per_second=2.4128G/s
   LongHashmapBenchmark_findEntryFromStdMap/100/process_time/real_time          
       0.414 ns        0.414 ns   1000000000 items_per_second=2.41719G/s
   LongHashmapBenchmark_findEntryFromStdMap/1000/process_time/real_time         
       0.414 ns        0.414 ns   1000000000 items_per_second=2.41582G/s
   LongHashmapBenchmark_findEntryFromStdMap/10000/process_time/real_time        
       0.414 ns        0.414 ns   1000000000 items_per_second=2.41681G/s
   LongHashmapBenchmark_findEntryFromStdMap/100000/process_time/real_time       
       0.414 ns        0.414 ns   1000000000 items_per_second=2.41521G/s
   LongHashmapBenchmark_findEntryFromStdMap/1000000/process_time/real_time      
       0.414 ns        0.414 ns   1000000000 items_per_second=2.41732G/s
   LongHashmapBenchmark_findEntryFromCelixMap/10/process_time/real_time         
        2.26 ns         2.26 ns    307909253 averageNrOfEntriesPerBucket=0.625 
items_per_second=441.585M/s nrOfBuckets=16 resizeCount=0 
stdDeviationNrOfEntriesPerBucket=0.856957
   LongHashmapBenchmark_findEntryFromCelixMap/100/process_time/real_time        
        3.39 ns         3.39 ns    211113789 averageNrOfEntriesPerBucket=3.125 
items_per_second=295.319M/s nrOfBuckets=32 resizeCount=1 
stdDeviationNrOfEntriesPerBucket=1.86665
   LongHashmapBenchmark_findEntryFromCelixMap/1000/process_time/real_time       
        2.48 ns         2.48 ns    281288298 
averageNrOfEntriesPerBucket=3.90625 items_per_second=403.557M/s nrOfBuckets=256 
resizeCount=4 stdDeviationNrOfEntriesPerBucket=2.16123
   LongHashmapBenchmark_findEntryFromCelixMap/10000/process_time/real_time      
        3.31 ns         3.31 ns    211375732 
averageNrOfEntriesPerBucket=4.88281 items_per_second=302.161M/s 
nrOfBuckets=2.048k resizeCount=7 stdDeviationNrOfEntriesPerBucket=2.2035
   LongHashmapBenchmark_findEntryFromCelixMap/100000/process_time/real_time     
        2.48 ns         2.48 ns    282405576 
averageNrOfEntriesPerBucket=3.75601 items_per_second=403.207M/s 
nrOfBuckets=26.624k resizeCount=11 stdDeviationNrOfEntriesPerBucket=1.92598
   LongHashmapBenchmark_findEntryFromCelixMap/1000000/process_time/real_time    
        2.90 ns         2.90 ns    241719478 
averageNrOfEntriesPerBucket=4.98246 items_per_second=344.381M/s 
nrOfBuckets=200.704k resizeCount=28 stdDeviationNrOfEntriesPerBucket=2.23011
   LongHashmapBenchmark_findEntryFromDeprecatedMap/10/process_time/real_time    
        3.54 ns         3.54 ns    197602959 items_per_second=282.865M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/100/process_time/real_time   
        3.54 ns         3.53 ns    197704646 items_per_second=282.885M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/1000/process_time/real_time  
        3.53 ns         3.53 ns    197684458 items_per_second=282.891M/s
   LongHashmapBenchmark_findEntryFromDeprecatedMap/10000/process_time/real_time 
        3.54 ns         3.54 ns    197731536 items_per_second=282.88M/s
   
LongHashmapBenchmark_findEntryFromDeprecatedMap/100000/process_time/real_time   
     3.54 ns         3.54 ns    197896830 items_per_second=282.817M/s
   
LongHashmapBenchmark_findEntryFromDeprecatedMap/1000000/process_time/real_time  
     3.54 ns         3.54 ns    198110019 items_per_second=282.856M/s
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to