Skip to content

Commit

Permalink
Eliminate hashTypeIterator memory allocation by assigning it on stack (…
Browse files Browse the repository at this point in the history 
…#1105)

Signed-off-by: Masahiro Ide <[email protected]>
Signed-off-by: Masahiro Ide <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
Co-authored-by: Masahiro Ide <[email protected]>
  • Loading branch information
@imasahiro @zuiderkwast
3 people authored Oct 6, 2024
1 parent a1cc7c2 commit b5eb793
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 53 deletions.
14 changes: 7 additions & 7 deletions src/aof.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1947,30 +1947,30 @@ static int rioWriteHashIteratorCursor(rio *r, hashTypeIterator *hi, int what) {
/* Emit the commands needed to rebuild a hash object.
* The function returns 0 on error, 1 on success. */
int rewriteHashObject(rio *r, robj *key, robj *o) {
hashTypeIterator *hi;
hashTypeIterator hi;
long long count = 0, items = hashTypeLength(o);

hi = hashTypeInitIterator(o);
while (hashTypeNext(hi) != C_ERR) {
hashTypeInitIterator(o, &hi);
while (hashTypeNext(&hi) != C_ERR) {
if (count == 0) {
int cmd_items = (items > AOF_REWRITE_ITEMS_PER_CMD) ? AOF_REWRITE_ITEMS_PER_CMD : items;

if (!rioWriteBulkCount(r, '*', 2 + cmd_items * 2) || !rioWriteBulkString(r, "HMSET", 5) ||
!rioWriteBulkObject(r, key)) {
hashTypeReleaseIterator(hi);
hashTypeResetIterator(&hi);
return 0;
}
}

if (!rioWriteHashIteratorCursor(r, hi, OBJ_HASH_KEY) || !rioWriteHashIteratorCursor(r, hi, OBJ_HASH_VALUE)) {
hashTypeReleaseIterator(hi);
if (!rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_KEY) || !rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_VALUE)) {
hashTypeResetIterator(&hi);
return 0;
}
if (++count == AOF_REWRITE_ITEMS_PER_CMD) count = 0;
items--;
}

hashTypeReleaseIterator(hi);
hashTypeResetIterator(&hi);

return 1;
}
Expand Down
11 changes: 6 additions & 5 deletions src/debug.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,21 +221,22 @@ void xorObjectDigest(serverDb *db, robj *keyobj, unsigned char *digest, robj *o)
serverPanic("Unknown sorted set encoding");
}
} else if (o->type == OBJ_HASH) {
hashTypeIterator *hi = hashTypeInitIterator(o);
while (hashTypeNext(hi) != C_ERR) {
hashTypeIterator hi;
hashTypeInitIterator(o, &hi);
while (hashTypeNext(&hi) != C_ERR) {
unsigned char eledigest[20];
sds sdsele;

memset(eledigest, 0, 20);
sdsele = hashTypeCurrentObjectNewSds(hi, OBJ_HASH_KEY);
sdsele = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_KEY);
mixDigest(eledigest, sdsele, sdslen(sdsele));
sdsfree(sdsele);
sdsele = hashTypeCurrentObjectNewSds(hi, OBJ_HASH_VALUE);
sdsele = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE);
mixDigest(eledigest, sdsele, sdslen(sdsele));
sdsfree(sdsele);
xorDigest(digest, eledigest, 20);
}
hashTypeReleaseIterator(hi);
hashTypeResetIterator(&hi);
} else if (o->type == OBJ_STREAM) {
streamIterator si;
streamIteratorStart(&si, o->ptr, NULL, NULL, 0);
Expand Down
6 changes: 3 additions & 3 deletions src/server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2603,7 +2603,7 @@ typedef struct {

unsigned char *fptr, *vptr;

dictIterator *di;
dictIterator di;
dictEntry *de;
} hashTypeIterator;

Expand Down Expand Up @@ -3370,8 +3370,8 @@ void hashTypeTryConversion(robj *subject, robj **argv, int start, int end);
int hashTypeExists(robj *o, sds key);
int hashTypeDelete(robj *o, sds key);
unsigned long hashTypeLength(const robj *o);
hashTypeIterator *hashTypeInitIterator(robj *subject);
void hashTypeReleaseIterator(hashTypeIterator *hi);
void hashTypeInitIterator(robj *subject, hashTypeIterator *hi);
void hashTypeResetIterator(hashTypeIterator *hi);
int hashTypeNext(hashTypeIterator *hi);
void hashTypeCurrentFromListpack(hashTypeIterator *hi,
int what,
Expand Down
75 changes: 37 additions & 38 deletions src/t_hash.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,25 +307,22 @@ unsigned long hashTypeLength(const robj *o) {
return length;
}

hashTypeIterator *hashTypeInitIterator(robj *subject) {
hashTypeIterator *hi = zmalloc(sizeof(hashTypeIterator));
void hashTypeInitIterator(robj *subject, hashTypeIterator *hi) {
hi->subject = subject;
hi->encoding = subject->encoding;

if (hi->encoding == OBJ_ENCODING_LISTPACK) {
hi->fptr = NULL;
hi->vptr = NULL;
} else if (hi->encoding == OBJ_ENCODING_HT) {
hi->di = dictGetIterator(subject->ptr);
dictInitIterator(&hi->di, subject->ptr);
} else {
serverPanic("Unknown hash encoding");
}
return hi;
}

void hashTypeReleaseIterator(hashTypeIterator *hi) {
if (hi->encoding == OBJ_ENCODING_HT) dictReleaseIterator(hi->di);
zfree(hi);
void hashTypeResetIterator(hashTypeIterator *hi) {
if (hi->encoding == OBJ_ENCODING_HT) dictResetIterator(&hi->di);
}

/* Move to the next entry in the hash. Return C_OK when the next entry
Expand Down Expand Up @@ -358,7 +355,7 @@ int hashTypeNext(hashTypeIterator *hi) {
hi->fptr = fptr;
hi->vptr = vptr;
} else if (hi->encoding == OBJ_ENCODING_HT) {
if ((hi->de = dictNext(hi->di)) == NULL) return C_ERR;
if ((hi->de = dictNext(&hi->di)) == NULL) return C_ERR;
} else {
serverPanic("Unknown hash encoding");
}
Expand Down Expand Up @@ -448,31 +445,31 @@ void hashTypeConvertListpack(robj *o, int enc) {
/* Nothing to do... */

} else if (enc == OBJ_ENCODING_HT) {
hashTypeIterator *hi;
hashTypeIterator hi;
dict *dict;
int ret;

hi = hashTypeInitIterator(o);
hashTypeInitIterator(o, &hi);
dict = dictCreate(&hashDictType);

/* Presize the dict to avoid rehashing */
dictExpand(dict, hashTypeLength(o));

while (hashTypeNext(hi) != C_ERR) {
while (hashTypeNext(&hi) != C_ERR) {
sds key, value;

key = hashTypeCurrentObjectNewSds(hi, OBJ_HASH_KEY);
value = hashTypeCurrentObjectNewSds(hi, OBJ_HASH_VALUE);
key = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_KEY);
value = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE);
ret = dictAdd(dict, key, value);
if (ret != DICT_OK) {
sdsfree(key);
sdsfree(value); /* Needed for gcc ASAN */
hashTypeReleaseIterator(hi); /* Needed for gcc ASAN */
sdsfree(value); /* Needed for gcc ASAN */
hashTypeResetIterator(&hi); /* Needed for gcc ASAN */
serverLogHexDump(LL_WARNING, "listpack with dup elements dump", o->ptr, lpBytes(o->ptr));
serverPanic("Listpack corruption detected");
}
}
hashTypeReleaseIterator(hi);
hashTypeResetIterator(&hi);
zfree(o->ptr);
o->encoding = OBJ_ENCODING_HT;
o->ptr = dict;
Expand All @@ -498,7 +495,7 @@ void hashTypeConvert(robj *o, int enc) {
* The resulting object always has refcount set to 1 */
robj *hashTypeDup(robj *o) {
robj *hobj;
hashTypeIterator *hi;
hashTypeIterator hi;

serverAssert(o->type == OBJ_HASH);

Expand All @@ -513,20 +510,20 @@ robj *hashTypeDup(robj *o) {
dict *d = dictCreate(&hashDictType);
dictExpand(d, dictSize((const dict *)o->ptr));

hi = hashTypeInitIterator(o);
while (hashTypeNext(hi) != C_ERR) {
hashTypeInitIterator(o, &hi);
while (hashTypeNext(&hi) != C_ERR) {
sds field, value;
sds newfield, newvalue;
/* Extract a field-value pair from an original hash object.*/
field = hashTypeCurrentFromHashTable(hi, OBJ_HASH_KEY);
value = hashTypeCurrentFromHashTable(hi, OBJ_HASH_VALUE);
field = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_KEY);
value = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_VALUE);
newfield = sdsdup(field);
newvalue = sdsdup(value);

/* Add a field-value pair to a new hash object. */
dictAdd(d, newfield, newvalue);
}
hashTypeReleaseIterator(hi);
hashTypeResetIterator(&hi);

hobj = createObject(OBJ_HASH, d);
hobj->encoding = OBJ_ENCODING_HT;
Expand Down Expand Up @@ -812,7 +809,7 @@ static void addHashIteratorCursorToReply(client *c, hashTypeIterator *hi, int wh

void genericHgetallCommand(client *c, int flags) {
robj *o;
hashTypeIterator *hi;
hashTypeIterator hi;
int length, count = 0;

robj *emptyResp = (flags & OBJ_HASH_KEY && flags & OBJ_HASH_VALUE) ? shared.emptymap[c->resp] : shared.emptyarray;
Expand All @@ -827,19 +824,19 @@ void genericHgetallCommand(client *c, int flags) {
addReplyArrayLen(c, length);
}

hi = hashTypeInitIterator(o);
while (hashTypeNext(hi) != C_ERR) {
hashTypeInitIterator(o, &hi);
while (hashTypeNext(&hi) != C_ERR) {
if (flags & OBJ_HASH_KEY) {
addHashIteratorCursorToReply(c, hi, OBJ_HASH_KEY);
addHashIteratorCursorToReply(c, &hi, OBJ_HASH_KEY);
count++;
}
if (flags & OBJ_HASH_VALUE) {
addHashIteratorCursorToReply(c, hi, OBJ_HASH_VALUE);
addHashIteratorCursorToReply(c, &hi, OBJ_HASH_VALUE);
count++;
}
}

hashTypeReleaseIterator(hi);
hashTypeResetIterator(&hi);

/* Make sure we returned the right number of elements. */
if (flags & OBJ_HASH_KEY && flags & OBJ_HASH_VALUE) count /= 2;
Expand Down Expand Up @@ -973,13 +970,14 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
* The number of requested elements is greater than the number of
* elements inside the hash: simply return the whole hash. */
if (count >= size) {
hashTypeIterator *hi = hashTypeInitIterator(hash);
while (hashTypeNext(hi) != C_ERR) {
hashTypeIterator hi;
hashTypeInitIterator(hash, &hi);
while (hashTypeNext(&hi) != C_ERR) {
if (withvalues && c->resp > 2) addReplyArrayLen(c, 2);
addHashIteratorCursorToReply(c, hi, OBJ_HASH_KEY);
if (withvalues) addHashIteratorCursorToReply(c, hi, OBJ_HASH_VALUE);
addHashIteratorCursorToReply(c, &hi, OBJ_HASH_KEY);
if (withvalues) addHashIteratorCursorToReply(c, &hi, OBJ_HASH_VALUE);
}
hashTypeReleaseIterator(hi);
hashTypeResetIterator(&hi);
return;
}

Expand Down Expand Up @@ -1015,21 +1013,22 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
/* Hashtable encoding (generic implementation) */
dict *d = dictCreate(&sdsReplyDictType);
dictExpand(d, size);
hashTypeIterator *hi = hashTypeInitIterator(hash);
hashTypeIterator hi;
hashTypeInitIterator(hash, &hi);

/* Add all the elements into the temporary dictionary. */
while ((hashTypeNext(hi)) != C_ERR) {
while ((hashTypeNext(&hi)) != C_ERR) {
int ret = DICT_ERR;
sds key, value = NULL;

key = hashTypeCurrentObjectNewSds(hi, OBJ_HASH_KEY);
if (withvalues) value = hashTypeCurrentObjectNewSds(hi, OBJ_HASH_VALUE);
key = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_KEY);
if (withvalues) value = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE);
ret = dictAdd(d, key, value);

serverAssert(ret == DICT_OK);
}
serverAssert(dictSize(d) == size);
hashTypeReleaseIterator(hi);
hashTypeResetIterator(&hi);

/* Remove random elements to reach the right count. */
while (size > count) {
Expand Down

0 comments on commit b5eb793

Please sign in to comment.