Repository: valkey-io/valkey
File: src/hashtable.c (C)

Code before:
    if (ht->pause_rehash > 0) return 0;
    if (resize_policy != HASHTABLE_RESIZE_ALLOW) return 0;

    monotime timer;
    elapsedStart(&timer);
    int rehashes = 0;

    while (hashtableIsRehashing(ht)) {
        rehashStep(ht);
        rehashes++;
        if (rehashes % 128 == 0 && elapsedUs(timer) >= us) break;
    }
    return rehashes;
}

/* Return true if expand was performed; false otherwise. */
bool hashtableExpand(hashtable *ht, size_t size) {
    return expand(ht, size, NULL);
}

/* Returns false if expand failed due to memory allocation failure. */
bool hashtableTryExpand(hashtable *ht, size_t size) {
    int malloc_failed = 0;
    expand(ht, size, &malloc_failed);
    return malloc_failed ? false : true;
}

/* Expanding is done automatically on insertion, but less eagerly if resize
 * policy is set to AVOID and not at all if set to FORBID.
 * Returns true if expanding, false if not expanding. */
bool hashtableExpandIfNeeded(hashtable *ht) {
    /* Don't expand if rehashing is already in progress. */
    if (hashtableIsRehashing(ht)) {
        return false;
    }

    size_t min_capacity = ht->used[0] + ht->used[1] + 1;
    size_t num_buckets = numBuckets(ht->bucket_exp[hashtableIsRehashing(ht) ? 1 : 0]);
    size_t current_capacity = num_buckets * ENTRIES_PER_BUCKET;
    unsigned max_fill_percent = resize_policy == HASHTABLE_RESIZE_ALLOW ? MAX_FILL_PERCENT_SOFT : MAX_FILL_PERCENT_HARD;
    if (min_capacity * 100 <= current_capacity * max_fill_percent) {
        return false;
    }
    return resize(ht, min_capacity, NULL);
}

/* Shrinking is done automatically on deletion, but less eagerly if resize
 * policy is set to AVOID and not at all if set to FORBID.
 * Returns true if shrinking, false if not shrinking. */
bool hashtableShrinkIfNeeded(hashtable *ht) {
    /* Don't shrink if rehashing is already in progress. */

Review comment (bug):
Why delete the documentation about what return true means? I think we should keep it.

Why change the logic? It is equivalent to the existing code, but is it simpler? I'm not sure. You ignore the return value of `expand`, which I think is less clear. I think we can return true if expand returns true and only look at `malloc_failed` if expand returned false.

If you really want to change this, then maybe like this?

```c
    if (expand(ht, size, &malloc_failed)) return true;
    return !malloc_failed;
```

Code after:
int hashtableRehashMicroseconds(hashtable *ht, uint64_t us) {
    if (ht->pause_rehash > 0) return 0;
    if (resize_policy != HASHTABLE_RESIZE_ALLOW) return 0;

    monotime timer;
    elapsedStart(&timer);
    int rehashes = 0;

    while (hashtableIsRehashing(ht)) {
        rehashStep(ht);
        rehashes++;
        if (rehashes % 128 == 0 && elapsedUs(timer) >= us) break;
    }
    return rehashes;
}

/* Return true if expand was performed; false otherwise. */
bool hashtableExpand(hashtable *ht, size_t size) {
    return expand(ht, size, NULL);
}

/* Returns true if expand was performed or if expand is not needed. Returns false if
 * expand failed due to memory allocation failure. */
bool hashtableTryExpand(hashtable *ht, size_t size) {
    int malloc_failed = 0;
    return expand(ht, size, &malloc_failed) || !malloc_failed;
}

/* Expanding is done automatically on insertion, but less eagerly if resize
 * policy is set to AVOID and not at all if set to FORBID.
 * Returns true if expanding, false if not expanding. */
bool hashtableExpandIfNeeded(hashtable *ht) {
    /* Don't expand if rehashing is already in progress. */
    if (hashtableIsRehashing(ht)) {
        return false;
    }

    size_t min_capacity = ht->used[0] + ht->used[1] + 1;
    size_t num_buckets = numBuckets(ht->bucket_exp[hashtableIsRehashing(ht) ? 1 : 0]);
    size_t current_capacity = num_buckets * ENTRIES_PER_BUCKET;
    unsigned max_fill_percent = resize_policy == HASHTABLE_RESIZE_ALLOW ? MAX_FILL_PERCENT_SOFT : MAX_FILL_PERCENT_HARD;
    if (min_capacity * 100 <= current_capacity * max_fill_percent) {
        return false;
    }
    return resize(ht, min_capacity, NULL);
}

/* Shrinking is done automatically on deletion, but less eagerly if resize
 * policy is set to AVOID and not at all if set to FORBID.
 * Returns true if shrinking, false if not shrinking. */
bool hashtableShrinkIfNeeded(hashtable *ht) {