Accelerate hash table iterator with value prefetching #1568
Conversation
NadavGigi
force-pushed
the
prefetch_values
branch
from
eb5feb4 to
1da6b36
Compare
Signed-off-by: NadavGigi <[email protected]>
NadavGigi
force-pushed
the
prefetch_values
branch
from
1da6b36 to
35f6b6e
Compare
NadavGigi
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #1568 +/- ##
============================================
- Coverage 70.78% 70.75% -0.04%
============================================
Files 120 120
Lines 65046 65079 +33
============================================
+ Hits 46045 46048 +3
- Misses 19001 19031 +30
|
| @@ -657,7 +657,7 @@ void ACLChangeSelectorPerm(aclSelector *selector, struct serverCommand *cmd, int | |||
| hashtableIterator iter; | |||
| hashtableInitSafeIterator(&iter, cmd->subcommands_ht); | |||
| void *next; | |||
| while (hashtableNext(&iter, &next)) { | |||
| while (hashtableNext(&iter, &next, 0)) { | |||
There was a problem hiding this comment.
Why aren't we prefetching in these cases?
There was a problem hiding this comment.
In this pr i have added support only for hashtables that storing robj and hashTypeEntry.
I will add support for more complex data structures in a different pr.
There was a problem hiding this comment.
This makes the hashtableNext call less simple and the change affects all places even where it is not used. I have another suggestions: storing the iterator flags in the iterator:
hashtableIterator iter;
hashtableInitSafeIterator(&iter, cmd->subcommands_ht);
hashtableIteratorSetFlags(HASHTABLE_ITER_PREFETCH_VALUES);
void *next;
while (hashtableNext(&iter, &next)) {Alternatively, we could set the prefetch callback directly in the iterator instead of storing it in the hashtableType.
hashtableIteratorSetPrefetchCallback(&iter, hashTypeEntryPrefetchValue);
There was a problem hiding this comment.
Interesting idea. I just skimmed though, not a full review.
To summarize, this is a second level of pre-fetching. Theoretically, we could do even more levels, right? It could even be generalized to any number of levels. We could use an array of prefetch callbacks.... (not saying we should)
We should probably also consider the bigger picture where this kind of callbacks can be used not only for the iterator, but also for the hashtableIncrementalFind that's already used with IO threads, and with other operations like Scan.
A hypothetical use case: Imagine we have a sorted set where we want to find the 10 members sorted by score, ZRANGE. We could find the starting element and also prefetch in up to 10 steps the next elements.
Have you been thinking about more prefetching ideas? Can you share your ideas? It's useful to understand the bigger picture better before we add many small optimizations for specific scenarios.
| @@ -657,7 +657,7 @@ void ACLChangeSelectorPerm(aclSelector *selector, struct serverCommand *cmd, int | |||
| hashtableIterator iter; | |||
| hashtableInitSafeIterator(&iter, cmd->subcommands_ht); | |||
| void *next; | |||
| while (hashtableNext(&iter, &next)) { | |||
| while (hashtableNext(&iter, &next, 0)) { | |||
There was a problem hiding this comment.
This makes the hashtableNext call less simple and the change affects all places even where it is not used. I have another suggestions: storing the iterator flags in the iterator:
hashtableIterator iter;
hashtableInitSafeIterator(&iter, cmd->subcommands_ht);
hashtableIteratorSetFlags(HASHTABLE_ITER_PREFETCH_VALUES);
void *next;
while (hashtableNext(&iter, &next)) {Alternatively, we could set the prefetch callback directly in the iterator instead of storing it in the hashtableType.
hashtableIteratorSetPrefetchCallback(&iter, hashTypeEntryPrefetchValue);
This PR builds upon the previous entry prefetching optimization to further enhance performance by implementing value prefetching for hashtable iterators.
Implementation
Modified
hashtableNextto accept a new flags parameter, allowing control over iterator behavior.Implemented conditional value prefetching within
hashtableNextbased on the newHASHTABLE_ITER_PREFETCH_VALUESflag.When the flag is set, hashtableNext now calls
prefetchBucketValuesat the start of each new bucket, preemptively loading the values of filled entries into the CPU cache.The actual prefetching of values is performed using type-specific callback functions implemented in
server.c:robjthehashtableObjectPrefetchValuecallback is used to prefetch the value if not embeded.hashTypeEntrythe hashHashtableTypePrefetchValue callback is used to prefetch the sds value.Updated relevant parts of the codebase (e.g., in
aof.c,debug.c,rdb.c) to utilize this new prefetching capability where appropriate.Currently supports value prefetching for hashtables storing
robjandhashTypeEntrytypes, laying the groundwork for future extensions to additional data types.Performance
Setup:
Action
Results
The results regarding the duration of “save” command was taken from “info all” command.
The results largely align with our expectations, showing significant improvements for larger values (100 bytes and 40 bytes) that are stored outside the robj. For smaller values (20 bytes and 10 bytes) that are embedded within the robj, we see almost no improvement, which is as expected.
However, the small improvement observed even for these embedded values is somewhat surprising. Given that we are not actively prefetching these embedded values, this minor performance gain was not anticipated.
perf record on save command without value prefetching:
perf record on save command with value prefetching:
Conclusions:
The prefetching strategy appears to be working as intended, shifting the performance bottleneck from data access to I/O operations.
The significant reduction in rdbSaveStringObject time suggests that string objects(which are the values) are being accessed more efficiently.