Unverified Commit c8548737 authored by Binbin's avatar Binbin Committed by GitHub
Browse files

Minor optimization in kvstoreDictAddRaw when dict exists (#13054)

Usually, the probability that a dict exists is much greater than the
probability that it does not exist. In kvstoreDictAddRaw, we will call
kvstoreGetDict multiple times. Based on this assumption, we change
createDictIfNeeded to something like get or create function:
```
before:
dict exist: 2 kvstoreGetDict
dict non-exist: 2 kvstoreGetDict

after:
dict exist: 1 kvstoreGetDict
dict non-exist: 3 kvstoreGetDict
```

A possible 3% performance improvement was observed:

In addition, some typos/comments i saw have been cleaned up.
parent 063de675
...@@ -155,11 +155,14 @@ static void cumulativeKeyCountAdd(kvstore *kvs, int didx, long delta) { ...@@ -155,11 +155,14 @@ static void cumulativeKeyCountAdd(kvstore *kvs, int didx, long delta) {
} }
} }
static void createDictIfNeeded(kvstore *kvs, int didx) { /* Create the dict if it does not exist and return it. */
if (kvstoreGetDict(kvs, didx)) static dict *createDictIfNeeded(kvstore *kvs, int didx) {
return; dict *d = kvstoreGetDict(kvs, didx);
if (d) return d;
kvs->dicts[didx] = dictCreate(&kvs->dtype); kvs->dicts[didx] = dictCreate(&kvs->dtype);
kvs->allocated_dicts++; kvs->allocated_dicts++;
return kvs->dicts[didx];
} }
static void freeDictIfNeeded(kvstore *kvs, int didx) { static void freeDictIfNeeded(kvstore *kvs, int didx) {
...@@ -236,7 +239,7 @@ kvstore *kvstoreCreate(dictType *type, int num_dicts_bits, int flags) { ...@@ -236,7 +239,7 @@ kvstore *kvstoreCreate(dictType *type, int num_dicts_bits, int flags) {
memcpy(&kvs->dtype, type, sizeof(kvs->dtype)); memcpy(&kvs->dtype, type, sizeof(kvs->dtype));
kvs->flags = flags; kvs->flags = flags;
/* kvstore must be the one to set this callbacks, so we make sure the /* kvstore must be the one to set these callbacks, so we make sure the
* caller didn't do it */ * caller didn't do it */
assert(!type->userdata); assert(!type->userdata);
assert(!type->dictMetadataBytes); assert(!type->dictMetadataBytes);
...@@ -402,7 +405,7 @@ unsigned long long kvstoreScan(kvstore *kvs, unsigned long long cursor, ...@@ -402,7 +405,7 @@ unsigned long long kvstoreScan(kvstore *kvs, unsigned long long cursor,
* Based on the parameter `try_expand`, appropriate dict expand API is invoked. * Based on the parameter `try_expand`, appropriate dict expand API is invoked.
* if try_expand is set to 1, `dictTryExpand` is used else `dictExpand`. * if try_expand is set to 1, `dictTryExpand` is used else `dictExpand`.
* The return code is either `DICT_OK`/`DICT_ERR` for both the API(s). * The return code is either `DICT_OK`/`DICT_ERR` for both the API(s).
* `DICT_OK` response is for successful expansion. However ,`DICT_ERR` response signifies failure in allocation in * `DICT_OK` response is for successful expansion. However, `DICT_ERR` response signifies failure in allocation in
* `dictTryExpand` call and in case of `dictExpand` call it signifies no expansion was performed. * `dictTryExpand` call and in case of `dictExpand` call it signifies no expansion was performed.
*/ */
int kvstoreExpand(kvstore *kvs, uint64_t newsize, int try_expand, kvstoreExpandShouldSkipDictIndex *skip_cb) { int kvstoreExpand(kvstore *kvs, uint64_t newsize, int try_expand, kvstoreExpandShouldSkipDictIndex *skip_cb) {
...@@ -581,7 +584,7 @@ dictEntry *kvstoreIteratorNext(kvstoreIterator *kvs_it) { ...@@ -581,7 +584,7 @@ dictEntry *kvstoreIteratorNext(kvstoreIterator *kvs_it) {
return de; return de;
} }
/* This method traverses through kvstore dictionaries and triggers a resize . /* This method traverses through kvstore dictionaries and triggers a resize.
* It first tries to shrink if needed, and if it isn't, it tries to expand. */ * It first tries to shrink if needed, and if it isn't, it tries to expand. */
void kvstoreTryResizeDicts(kvstore *kvs, int limit) { void kvstoreTryResizeDicts(kvstore *kvs, int limit) {
if (limit > kvs->num_dicts) if (limit > kvs->num_dicts)
...@@ -599,7 +602,7 @@ void kvstoreTryResizeDicts(kvstore *kvs, int limit) { ...@@ -599,7 +602,7 @@ void kvstoreTryResizeDicts(kvstore *kvs, int limit) {
/* Our hash table implementation performs rehashing incrementally while /* Our hash table implementation performs rehashing incrementally while
* we write/read from the hash table. Still if the server is idle, the hash * we write/read from the hash table. Still if the server is idle, the hash
* table will use two tables for a long time. So we try to use 1 millisecond * table will use two tables for a long time. So we try to use threshold_us
* of CPU time at every call of this function to perform some rehashing. * of CPU time at every call of this function to perform some rehashing.
* *
* The function returns the amount of microsecs spent if some rehashing was * The function returns the amount of microsecs spent if some rehashing was
...@@ -608,7 +611,7 @@ uint64_t kvstoreIncrementallyRehash(kvstore *kvs, uint64_t threshold_us) { ...@@ -608,7 +611,7 @@ uint64_t kvstoreIncrementallyRehash(kvstore *kvs, uint64_t threshold_us) {
if (listLength(kvs->rehashing) == 0) if (listLength(kvs->rehashing) == 0)
return 0; return 0;
/* Our goal is to rehash as many dictionaries as we can before reaching predefined threshold, /* Our goal is to rehash as many dictionaries as we can before reaching threshold_us,
* after each dictionary completes rehashing, it removes itself from the list. */ * after each dictionary completes rehashing, it removes itself from the list. */
listNode *node; listNode *node;
monotime timer; monotime timer;
...@@ -754,8 +757,7 @@ dictEntry *kvstoreDictFind(kvstore *kvs, int didx, void *key) { ...@@ -754,8 +757,7 @@ dictEntry *kvstoreDictFind(kvstore *kvs, int didx, void *key) {
} }
dictEntry *kvstoreDictAddRaw(kvstore *kvs, int didx, void *key, dictEntry **existing) { dictEntry *kvstoreDictAddRaw(kvstore *kvs, int didx, void *key, dictEntry **existing) {
createDictIfNeeded(kvs, didx); dict *d = createDictIfNeeded(kvs, didx);
dict *d = kvstoreGetDict(kvs, didx);
dictEntry *ret = dictAddRaw(d, key, existing); dictEntry *ret = dictAddRaw(d, key, existing);
if (ret) if (ret)
cumulativeKeyCountAdd(kvs, didx, 1); cumulativeKeyCountAdd(kvs, didx, 1);
......
...@@ -42,7 +42,7 @@ dictEntry *kvstoreIteratorNext(kvstoreIterator *kvs_it); ...@@ -42,7 +42,7 @@ dictEntry *kvstoreIteratorNext(kvstoreIterator *kvs_it);
/* Rehashing */ /* Rehashing */
void kvstoreTryResizeDicts(kvstore *kvs, int limit); void kvstoreTryResizeDicts(kvstore *kvs, int limit);
uint64_t kvstoreIncrementallyRehash(kvstore *kvs, uint64_t threshold_ms); uint64_t kvstoreIncrementallyRehash(kvstore *kvs, uint64_t threshold_us);
/* Specific dict access by dict-index */ /* Specific dict access by dict-index */
unsigned long kvstoreDictSize(kvstore *kvs, int didx); unsigned long kvstoreDictSize(kvstore *kvs, int didx);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment