PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1390973000
##########
libs/utils/src/celix_hash_map.c:
##########
@@ -19,64 +19,56 @@
#include "celix_string_hash_map.h"
#include "celix_long_hash_map.h"
-#include "celix_utils.h"
+#include "celix_hash_map_private.h"
+#include "celix_hash_map_internal.h"
#include <stdlib.h>
#include <memory.h>
#include <math.h>
#include <assert.h>
#include <stdint.h>
-/**
- * Whether to use realloc - instead of calloc - to resize a hash map.
- *
- * With realloc the memory is increased and the
- * map entries are corrected.
- *
- * With alloc a new buckets array is allocated and
- * entries are moved into the new bucket array.
- * And the old buckets array is freed.
- *
- */
-#define CELIX_HASH_MAP_RESIZE_WITH_REALLOC
-
-static unsigned int DEFAULT_INITIAL_CAPACITY = 16;
-static double DEFAULT_LOAD_FACTOR = 0.75;
-static unsigned int MAXIMUM_CAPACITY = INT32_MAX/10;
+#include "celix_utils.h"
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
-typedef enum celix_hash_map_key_type {
- CELIX_HASH_MAP_STRING_KEY,
- CELIX_HASH_MAP_LONG_KEY
-} celix_hash_map_key_type_e;
+#define CELIX_HASHMAP_DEFAULT_INITIAL_CAPACITY 16
+#define CELIX_HASHMAP_DEFAULT_MAX_LOAD_FACTOR 5.0
+#define CELIX_HASHMAP_CAPACITY_INCREASE_FACTOR 2
+#define CELIX_HASHMAP_MAXIMUM_CAPACITY (INT32_MAX/10)
+#define CELIX_HASHMAP_MAXIMUM_INCREASE_VALUE (1024*10)
+#define CELIX_HASHMAP_HASH_PRIME 1610612741
-typedef union celix_hash_map_key {
+union celix_hash_map_key {
const char* strKey;
long longKey;
-} celix_hash_map_key_t;
+};
-typedef struct celix_hash_map_entry celix_hash_map_entry_t;
struct celix_hash_map_entry {
celix_hash_map_key_t key;
celix_hash_map_value_t value;
celix_hash_map_entry_t* next;
unsigned int hash;
};
-typedef struct celix_hash_map {
+struct celix_hash_map {
celix_hash_map_entry_t** buckets;
unsigned int bucketsSize; //nr of buckets
unsigned int size; //nr of total entries
- double loadFactor;
+ double maxLoadFactor;
celix_hash_map_key_type_e keyType;
- celix_hash_map_value_t emptyValue;
- unsigned int (*hashKeyFunction)(const celix_hash_map_key_t* key);
- bool (*equalsKeyFunction)(const celix_hash_map_key_t* key1, const
celix_hash_map_key_t* key2);
void (*simpleRemovedCallback)(void* value);
void* removedCallbackData;
- void (*removedStringKeyCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue);
- void (*removedLongKeyCallback)(void* data, long removedKey,
celix_hash_map_value_t removedValue);
+ void (*removedStringEntryCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue);
+ void (*removedStringKeyCallback)(void* data, char* key);
+ void (*removedLongEntryCallback)(void* data, long removedKey,
celix_hash_map_value_t removedValue);
bool storeKeysWeakly;
-} celix_hash_map_t;
+
+ //statistics
+ size_t modificationsCount;
+ size_t resizeCount;
+ size_t collisionsCount;
Review Comment:
`modificationsCount` and `collisionsCount` are unused now.
##########
libs/utils/include/celix_long_hash_map.h:
##########
@@ -105,21 +114,21 @@ typedef struct celix_long_hash_map_create_options {
unsigned int initialCapacity CELIX_OPTS_INIT;
/**
- * @brief The hash map load factor, which controls the max ratio between
nr of entries in the hash map and the
+ * @brief The hash map max load factor, which controls the max ratio
between nr of entries in the hash map and the
* hash map capacity.
*
- * The load factor controls how large the hash map capacity (nr of
buckets) is compared to the nr of entries
+ * The max load factor controls how large the hash map capacity (nr of
buckets) is compared to the nr of entries
* in the hash map. The load factor is an important property of the hash
map which influences how close the
* hash map performs to O(1) for its get, has and put operations.
*
- * If the nr of entries increases above the loadFactor * capacity, the
hash capacity will be doubled.
+ * If the nr of entries increases above the maxLoadFactor * capacity, the
hash table capacity will be doubled.
* For example a hash map with capacity 16 and load factor 0.75 will
double its capacity when the 13th entry
* is added to the hash map.
*
* If 0 is provided, the hash map load factor will be 0.75 (default hash
map load factor).
Review Comment:
```suggestion
* If 0 is provided, the hash map load factor will be 5 (default hash
map load factor).
```
##########
libs/utils/src/celix_hash_map.c:
##########
@@ -599,33 +675,49 @@ bool celix_longHashMapIterator_isEnd(const
celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
+ iter->index += 1;
Review Comment:
I agree that "the index of the end iterator is the same as the size of the
map".
The current implementation of `celix_stringHashMapIterator_next` allow
`iter->index > map->genericMap.size`. Is is intended?
--
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
