Skip to content

Commit aac3c76

Browse files
authored
Fix prefetchNextBucketEntries so it fetches the correct next 2 buckets (#2394)
This PR fixes a bug in prefetchNextBucketEntries which is used in the hashtable iterator. The current version of prefetchNextBucketEntries does not correctly prefetch the next two buckets that will be iterated over. This is due next_index being incremented twice (once in prefetchNextBucketEntries and again in getNextBucket). Fix: 1. Add check in prefetchNextBucketEntries to ensure we pass the correct 'next_index' to getNextBucket. 2. The extra increment was removed from getNextBucket. Signed-off-by: Nicky Khorasani <[email protected]>
1 parent dceb9f3 commit aac3c76

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

src/hashtable.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -992,16 +992,16 @@ static void prefetchBucketEntries(bucket *b) {
992992
}
993993
}
994994

995-
/* Returns the child bucket if chained, otherwise the next bucket in the table. returns NULL if neither exists. */
996-
static bucket *getNextBucket(bucket *current_bucket, size_t bucket_index, hashtable *ht, int table_index) {
995+
/* Returns the child bucket if 'current_bucket' is chained. Otherwise, returns the bucket
996+
* at 'next_top_level_index' in the table. Returns NULL if neither exists. */
997+
static bucket *getNextBucket(bucket *current_bucket, size_t next_top_level_index, hashtable *ht, int table_index) {
997998
bucket *next_bucket = NULL;
998999
if (current_bucket->chained) {
9991000
next_bucket = getChildBucket(current_bucket);
10001001
} else {
10011002
size_t table_size = numBuckets(ht->bucket_exp[table_index]);
1002-
size_t next_index = bucket_index + 1;
1003-
if (next_index < table_size) {
1004-
next_bucket = &ht->tables[table_index][next_index];
1003+
if (next_top_level_index < table_size) {
1004+
next_bucket = &ht->tables[table_index][next_top_level_index];
10051005
}
10061006
}
10071007
return next_bucket;
@@ -1012,7 +1012,7 @@ static bucket *getNextBucket(bucket *current_bucket, size_t bucket_index, hashta
10121012
* - The next of the next bucket
10131013
* It attempts to bring this data closer to the L1 cache to reduce future memory access latency.
10141014
*
1015-
* Cache state before this function is called(due to last call for this function):
1015+
* Cache state before this function is called (due to last call for this function):
10161016
* 1. The current bucket and its entries are likely already in cache.
10171017
* 2. The next bucket is in cache.
10181018
*/
@@ -1021,7 +1021,9 @@ static void prefetchNextBucketEntries(iter *iter, bucket *current_bucket) {
10211021
bucket *next_bucket = getNextBucket(current_bucket, next_index, iter->hashtable, iter->table);
10221022
if (next_bucket) {
10231023
prefetchBucketEntries(next_bucket);
1024-
bucket *next_next_bucket = getNextBucket(next_bucket, next_index + 1, iter->hashtable, iter->table);
1024+
/* Calculate the target top-level index for the next-next bucket. */
1025+
if (!current_bucket->chained) next_index++;
1026+
bucket *next_next_bucket = getNextBucket(next_bucket, next_index, iter->hashtable, iter->table);
10251027
if (next_next_bucket) {
10261028
valkey_prefetch(next_next_bucket);
10271029
}

0 commit comments

Comments
 (0)