diff --git a/src/borghash/HashTable.pxd b/src/borghash/HashTable.pxd index d00222c..169f32a 100644 --- a/src/borghash/HashTable.pxd +++ b/src/borghash/HashTable.pxd @@ -4,7 +4,7 @@ cdef class HashTable: cdef int ksize, vsize cdef readonly size_t capacity, used cdef size_t initial_capacity, tombstones - cdef float max_load_factor, min_load_factor, shrink_factor, grow_factor + cdef float max_load_factor, min_load_factor, shrink_factor, grow_factor, rehash_threshold cdef uint32_t* table cdef uint32_t kv_capacity, kv_used cdef float kv_grow_factor @@ -14,6 +14,6 @@ cdef class HashTable: cdef int stats_resize_table, stats_resize_kv cdef size_t _get_index(self, uint8_t* key) - cdef int _lookup_index(self, uint8_t* key_ptr, size_t* index_ptr) + cdef int _lookup_index(self, uint8_t* key_ptr, size_t* index_ptr, size_t* tombstone_ptr = *) cdef void _resize_table(self, size_t new_capacity) cdef void _resize_kv(self, size_t new_capacity) diff --git a/src/borghash/HashTable.pyx b/src/borghash/HashTable.pyx index 1fcd1a8..5811fe7 100644 --- a/src/borghash/HashTable.pyx +++ b/src/borghash/HashTable.pyx @@ -47,12 +47,15 @@ cdef class HashTable: key_size: int = 0, value_size: int = 0, capacity: int = MIN_CAPACITY, max_load_factor: float = 0.5, min_load_factor: float = 0.10, shrink_factor: float = 0.4, grow_factor: float = 2.0, - kv_grow_factor: float = 1.3) -> None: + rehash_threshold: float = 0.75, kv_grow_factor: float = 1.3) -> None: # the load of the ht (.table) shall be between 0.25 and 0.5, so it is fast and has few collisions. # it is cheap to have a low hash table load, because .table only stores uint32_t indices into the # .keys and .values array. # the keys/values arrays have bigger elements and are not hash tables, thus collisions and load # factor are no concern there. the kv_grow_factor can be relatively small. + # tombstones count towards the load of the ht, but unlike used entries they can be dropped by + # rehashing at the same capacity. rehash_threshold decides between the two: only grow the ht if + # the used entries alone occupy more than that fraction of the load budget. if key_size < 4: raise ValueError("key_size must be specified and must be >= 4.") if not value_size: @@ -64,6 +67,7 @@ cdef class HashTable: self.min_load_factor = min_load_factor self.shrink_factor = shrink_factor self.grow_factor = grow_factor + self.rehash_threshold = rehash_threshold self.initial_capacity = capacity self.capacity = 0 self.used = 0 @@ -111,24 +115,32 @@ cdef class HashTable: cdef uint32_t key32 = (key[0] << 24) | (key[1] << 16) | (key[2] << 8) | key[3] return key32 % self.capacity - cdef int _lookup_index(self, uint8_t* key_ptr, size_t* index_ptr): + cdef int _lookup_index(self, uint8_t* key_ptr, size_t* index_ptr, size_t* tombstone_ptr = NULL): """ search for a specific key. if found, return 1 and set *index_ptr to the index of the bucket in self.table. if not found, return 0 and set *index_ptr to the index of a free bucket in self.table. + if not found and tombstone_ptr is given, set *tombstone_ptr to the index of the first + tombstone bucket that was passed on the way, or to self.capacity if there was none. """ cdef size_t index = self._get_index(key_ptr) + cdef size_t first_tombstone = self.capacity # == "none", indices are < self.capacity cdef uint32_t kv_index self.stats_lookup += 1 while (kv_index := self.table[index]) != FREE_BUCKET: self.stats_linear += 1 - if kv_index != TOMBSTONE_BUCKET and memcmp(self.keys + kv_index * self.ksize, key_ptr, self.ksize) == 0: + if kv_index == TOMBSTONE_BUCKET: + if first_tombstone == self.capacity: + first_tombstone = index + elif memcmp(self.keys + kv_index * self.ksize, key_ptr, self.ksize) == 0: if index_ptr: index_ptr[0] = index return 1 # found index = (index + 1) % self.capacity if index_ptr: index_ptr[0] = index + if tombstone_ptr: + tombstone_ptr[0] = first_tombstone return 0 # not found def __setitem__(self, key: bytes, value: bytes) -> None: @@ -138,9 +150,9 @@ cdef class HashTable: cdef uint8_t* key_ptr = key cdef uint8_t* value_ptr = value cdef uint32_t kv_index - cdef size_t index + cdef size_t index, tombstone_index self.stats_set += 1 - if self._lookup_index(key_ptr, &index): + if self._lookup_index(key_ptr, &index, &tombstone_index): kv_index = self.table[index] memcpy(self.values + kv_index * self.vsize, value_ptr, self.vsize) return @@ -159,10 +171,16 @@ cdef class HashTable: self.kv_used += 1 self.used += 1 + if tombstone_index < self.capacity: # prefer a tombstone bucket, it is dead weight otherwise + index = tombstone_index + self.tombstones -= 1 self.table[index] = kv_index # _lookup_index has set index to a free bucket if self.used + self.tombstones > self.capacity * self.max_load_factor: - self._resize_table(int(self.capacity * self.grow_factor)) + if self.used > self.capacity * self.max_load_factor * self.rehash_threshold: + self._resize_table(int(self.capacity * self.grow_factor)) + else: # mostly tombstones: rehashing at the same capacity drops them + self._resize_table(self.capacity) def __contains__(self, key: bytes) -> bool: if len(key) != self.ksize: @@ -200,7 +218,8 @@ cdef class HashTable: # Resize down if necessary if self.used < self.capacity * self.min_load_factor: new_capacity = max(int(self.capacity * self.shrink_factor), MIN_CAPACITY) - self._resize_table(new_capacity) + if new_capacity < self.capacity: # at the MIN_CAPACITY floor, this would rehash per delete + self._resize_table(new_capacity) else: raise KeyError("Key not found") diff --git a/tests/hashtable_test.py b/tests/hashtable_test.py index 7cf6ce4..18be743 100644 --- a/tests/hashtable_test.py +++ b/tests/hashtable_test.py @@ -1,4 +1,5 @@ import hashlib +import struct import pytest @@ -174,6 +175,68 @@ def test_stats(ht): assert ht.stats["iter"] == 1 +def test_delete_at_min_capacity_does_not_rehash(ht): + # a table that is already at the MIN_CAPACITY floor must not rehash on every delete + keys = [H2(i) for i in range(200)] + for key in keys: + ht[key] = value1 + capacity, resizes = ht.capacity, ht.stats["resize_table"] + for key in keys: + del ht[key] + assert len(ht) == 0 + assert ht.capacity == capacity + assert ht.stats["resize_table"] == resizes + + +def test_tombstones_do_not_grow_table(): + # hitting the load threshold with mostly tombstones must rehash at the same capacity, not grow + ht = HashTable(key_size=32, value_size=4, capacity=100000) + keys = [H2(i) for i in range(60000)] + for key in keys[:40000]: + ht[key] = value1 + for key in keys[:25000]: # 25000 tombstones, 15000 entries left + del ht[key] + assert ht.capacity == 100000 + for key in keys[40000:]: # 20000 new entries, crossing max_load_factor on the way + ht[key] = value1 + assert len(ht) == 35000 + assert ht.capacity == 100000 + assert set(ht.items()) == {(key, value1) for key in keys[25000:]} + + +def test_tombstone_is_recycled(): + def K(i): + # _get_index is key32 % capacity, so these all want bucket 7 and form one probe chain + return struct.pack(">I", 1000 * i + 7) + bytes(28) + + ht = HashTable(key_size=32, value_size=4, capacity=1000) + for i in range(3): + ht[K(i)] = value1 # buckets 7, 8, 9 + del ht[K(1)] # bucket 8 becomes a tombstone + ht[K(3)] = value1 # must reuse bucket 8, not take bucket 10 + linear = ht.stats["linear"] + assert ht[K(3)] == value1 + assert ht.stats["linear"] - linear == 2 # visited buckets 7 and 8 + + +def test_used_entries_grow_table(): + ht = HashTable(key_size=32, value_size=4, capacity=1000) + for i in range(600): # more than capacity * max_load_factor + ht[H2(i)] = value1 + assert ht.capacity > 1000 + + +def test_delete_shrinks_table(): + ht = HashTable(key_size=32, value_size=4, capacity=100000) + keys = [H2(i) for i in range(20000)] + for key in keys: + ht[key] = value1 + assert ht.capacity == 100000 + for key in keys[:19000]: + del ht[key] + assert ht.capacity < 100000 + + def test_k_to_idx(ht12): idx1 = ht12.k_to_idx(key1) idx2 = ht12.k_to_idx(key2)