Browse code

Fix hash impl when replacing values.

We never remove keys from the hash, we only replace keys that expire due to LRU,
with the new keys. However the new key is not inserted in the same place as the
old one! (it depends where it hashes) So we need to mark the old key as deleted,
and not as empty. Otherwise future searches could miss keys that are in the
hash!

Török Edvin authored on 2010/01/07 02:53:07
Showing 1 changed files
... ...
@@ -182,6 +182,10 @@ int cache_check(unsigned char *hash, cli_ctx *ctx) {
182 182
     cli_md5_final(hash, &md5);
183 183
     return cache_lookup_hash(hash, ctx);
184 184
 }
185
+#if 0
186
+#define cli_calloc calloc
187
+#define cli_errmsg(x)
188
+#endif
185 189
 
186 190
 struct cache_key {
187 191
     char digest[16];
... ...
@@ -200,6 +204,8 @@ struct cache_set {
200 200
 };
201 201
 
202 202
 #define CACHE_INVALID_VERSION ~0u
203
+#define CACHE_KEY_DELETED ~0u
204
+#define CACHE_KEY_EMPTY 0
203 205
 
204 206
 /* size must be power of 2! */
205 207
 static int cacheset_init(struct cache_set* map, size_t maxsize, uint8_t loadfactor)
... ...
@@ -250,7 +256,17 @@ static void cacheset_lru_remove(struct cache_set *map, size_t howmany)
250 250
 	// Remove a key from the head of the list
251 251
 	old = map->lru_head;
252 252
 	map->lru_head = old->lru_next;
253
-	old->size = 0; /* this slot is now empty */
253
+	old->size = CACHE_KEY_DELETED;
254
+	/* This slot is now deleted, it is not empty,
255
+	 * because previously we could have inserted a key that has seen this
256
+	 * slot as occupied, to find that key we need to ensure that all keys
257
+	 * that were occupied when the key was inserted, are seen as occupied
258
+	 * when searching too.
259
+	 * Of course when inserting a new value, we treat deleted slots as
260
+	 * empty.
261
+	 * We only replace old values with new values, but there is no guarantee
262
+	 * that the newly inserted value would hash to same place as the value
263
+	 * we remove due to LRU! */
254 264
 	if (old == map->lru_tail)
255 265
 	    map->lru_tail = 0;
256 266
     }
... ...
@@ -282,18 +298,23 @@ static inline size_t hash(const unsigned char* k,const size_t len,const size_t S
282 282
 }
283 283
 
284 284
 int cacheset_lookup_internal(struct cache_set *map, const struct cache_key *key,
285
-			     uint32_t *insert_pos)
285
+			     uint32_t *insert_pos, int deletedok)
286 286
 {
287 287
     uint32_t idx = hash((const unsigned char*)key, sizeof(*key), map->capacity);
288 288
     uint32_t tries = 0;
289 289
     struct cache_key *k = &map->data[idx];
290
-    while (k->size) {
290
+    while (k->size != CACHE_KEY_EMPTY) {
291 291
 	if (k->size == key->size &&
292 292
 	    !memcmp(k->digest, key, 16)) {
293 293
 	    /* found key */
294 294
 	    *insert_pos = idx;
295 295
 	    return 1;
296 296
 	}
297
+       if (deletedok && k->size == CACHE_KEY_DELETED) {
298
+           /* treat deleted slot as empty */
299
+           *insert_pos = idx;
300
+           return 0;
301
+       }
297 302
 	idx = (idx + tries++)&(map->capacity-1);
298 303
 	k = &map->data[idx];
299 304
     }
... ...
@@ -332,7 +353,7 @@ static void cacheset_add(struct cache_set *map, const struct cache_key *key)
332 332
 	cacheset_lru_remove(map, 1);
333 333
     assert(map->elements < map->maxelements);
334 334
 
335
-    ret = cacheset_lookup_internal(map, key, &pos);
335
+    ret = cacheset_lookup_internal(map, key, &pos, 1);
336 336
     newkey = &map->data[pos];
337 337
     if (ret) {
338 338
 	/* was already added, remove from LRU list */
... ...
@@ -353,7 +374,7 @@ static int cacheset_lookup(struct cache_set *map, const struct cache_key *key)
353 353
     struct cache_key *newkey;
354 354
     int ret;
355 355
     uint32_t pos;
356
-    ret = cacheset_lookup_internal(map, key, &pos);
356
+    ret = cacheset_lookup_internal(map, key, &pos, 0);
357 357
     if (!ret)
358 358
 	return CACHE_INVALID_VERSION;
359 359
     newkey = &map->data[pos];