Skip to content

Commit 9f35f81

Browse files
committed
Improve Hashtable buffer grow and shrink conditions
* Move assertions about hash table sizes to Hashtable_isConsistent() so they can be checked in all Hashtable methods. * Slightly improve conditionals of growing and shrinking the "buckets" buffer. Specifically the calculations are now less prone to arithmetic overflow and can work with Hashtable.size value up to (SIZE_MAX / 7). (Original limit was (SIZE_MAX / 10)). * If `Hashtable.size > SIZE_MAX / sizeof(HashtableItem)`, allow the compiler to optimize out one conditional of checking overflow. (The buffer allocation would still fail at xCalloc() in that case.) * Hashtable_setSize() is now a private method. Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
1 parent 69813bb commit 9f35f81

2 files changed

Lines changed: 20 additions & 20 deletions

File tree

Hashtable.c

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ static bool Hashtable_isConsistent(const Hashtable* this) {
7474
bool res = items == this->items;
7575
if (!res)
7676
Hashtable_dump(this);
77+
78+
assert(this->size > 0);
79+
assert(this->size <= SIZE_MAX / sizeof(HashtableItem));
80+
assert(this->size >= this->items);
81+
7782
return res;
7883
}
7984

@@ -193,21 +198,18 @@ static void insert(Hashtable* this, ht_key_t key, void* value) {
193198
}
194199
}
195200

196-
void Hashtable_setSize(Hashtable* this, size_t size) {
197-
201+
static void Hashtable_setSize(Hashtable* this, size_t size) {
198202
assert(Hashtable_isConsistent(this));
203+
assert(size >= this->items);
199204

200-
if (size <= this->items)
201-
return;
202-
203-
size_t newSize = nextPrime(size);
204-
if (newSize == this->size)
205+
size = nextPrime(size);
206+
if (size == this->size)
205207
return;
206208

207209
HashtableItem* oldBuckets = this->buckets;
208210
size_t oldSize = this->size;
209211

210-
this->size = newSize;
212+
this->size = size;
211213
this->buckets = (HashtableItem*) xCalloc(this->size, sizeof(HashtableItem));
212214
this->items = 0;
213215

@@ -225,24 +227,20 @@ void Hashtable_setSize(Hashtable* this, size_t size) {
225227
}
226228

227229
void Hashtable_put(Hashtable* this, ht_key_t key, void* value) {
228-
229230
assert(Hashtable_isConsistent(this));
230-
assert(this->size > 0);
231231
assert(value);
232232

233233
/* grow on load-factor > 0.7 */
234-
if (10 * this->items > 7 * this->size) {
235-
if (SIZE_MAX / 2 < this->size)
236-
CRT_fatalError("Hashtable: size overflow");
234+
if (sizeof(HashtableItem) < 7 && SIZE_MAX / 7 < this->size)
235+
CRT_fatalError("Hashtable: size overflow");
237236

238-
Hashtable_setSize(this, 2 * this->size);
239-
}
237+
if (this->items >= this->size * 7 / 10)
238+
Hashtable_setSize(this, this->size + 1);
240239

241240
insert(this, key, value);
242241

243242
assert(Hashtable_isConsistent(this));
244243
assert(Hashtable_get(this, key) != NULL);
245-
assert(this->size > this->items);
246244
}
247245

248246
void* Hashtable_remove(Hashtable* this, ht_key_t key) {
@@ -294,8 +292,12 @@ void* Hashtable_remove(Hashtable* this, ht_key_t key) {
294292
assert(Hashtable_get(this, key) == NULL);
295293

296294
/* shrink on load-factor < 0.125 */
297-
if (8 * this->items < this->size)
298-
Hashtable_setSize(this, this->size / 3); /* account for nextPrime rounding up */
295+
if (sizeof(HashtableItem) < 3 && SIZE_MAX / 3 < this->size)
296+
CRT_fatalError("Hashtable: size overflow");
297+
298+
if (this->items < this->size / 8) {
299+
Hashtable_setSize(this, this->size * 3 / 8); /* account for nextPrime rounding up */
300+
}
299301

300302
return res;
301303
}

Hashtable.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ void Hashtable_delete(Hashtable* this);
2929

3030
void Hashtable_clear(Hashtable* this);
3131

32-
void Hashtable_setSize(Hashtable* this, size_t size);
33-
3432
void Hashtable_put(Hashtable* this, ht_key_t key, void* value);
3533

3634
void* Hashtable_remove(Hashtable* this, ht_key_t key);

0 commit comments

Comments
 (0)