Commit 69ef89f2 authored by Pieter Noordhuis's avatar Pieter Noordhuis
Browse files

Reference zset score in zskiplistNode from dict entries

This avoids the extra allocation of sizeof(double) for storing the score
of a zset entry in the hash table. Saves sizeof(double) + malloc
overhead = approx. 16 bytes per zset entry.
parent 2159782b
...@@ -338,7 +338,7 @@ dictType zsetDictType = { ...@@ -338,7 +338,7 @@ dictType zsetDictType = {
NULL, /* val dup */ NULL, /* val dup */
dictEncObjKeyCompare, /* key compare */ dictEncObjKeyCompare, /* key compare */
dictRedisObjectDestructor, /* key destructor */ dictRedisObjectDestructor, /* key destructor */
dictVanillaFree /* val destructor of malloc(sizeof(double)) */ NULL /* val destructor */
}; };
/* Db->dict, keys are sds strings, vals are Redis objects. */ /* Db->dict, keys are sds strings, vals are Redis objects. */
......
...@@ -675,7 +675,7 @@ void backgroundRewriteDoneHandler(int statloc); ...@@ -675,7 +675,7 @@ void backgroundRewriteDoneHandler(int statloc);
/* Sorted sets data type */ /* Sorted sets data type */
zskiplist *zslCreate(void); zskiplist *zslCreate(void);
void zslFree(zskiplist *zsl); void zslFree(zskiplist *zsl);
void zslInsert(zskiplist *zsl, double score, robj *obj); zskiplistNode *zslInsert(zskiplist *zsl, double score, robj *obj);
/* Core functions */ /* Core functions */
void freeMemoryIfNeeded(void); void freeMemoryIfNeeded(void);
......
...@@ -71,7 +71,7 @@ int zslRandomLevel(void) { ...@@ -71,7 +71,7 @@ int zslRandomLevel(void) {
return (level<ZSKIPLIST_MAXLEVEL) ? level : ZSKIPLIST_MAXLEVEL; return (level<ZSKIPLIST_MAXLEVEL) ? level : ZSKIPLIST_MAXLEVEL;
} }
void zslInsert(zskiplist *zsl, double score, robj *obj) { zskiplistNode *zslInsert(zskiplist *zsl, double score, robj *obj) {
zskiplistNode *update[ZSKIPLIST_MAXLEVEL], *x; zskiplistNode *update[ZSKIPLIST_MAXLEVEL], *x;
unsigned int rank[ZSKIPLIST_MAXLEVEL]; unsigned int rank[ZSKIPLIST_MAXLEVEL];
int i, level; int i, level;
...@@ -123,6 +123,7 @@ void zslInsert(zskiplist *zsl, double score, robj *obj) { ...@@ -123,6 +123,7 @@ void zslInsert(zskiplist *zsl, double score, robj *obj) {
else else
zsl->tail = x; zsl->tail = x;
zsl->length++; zsl->length++;
return x;
} }
/* Internal function used by zslDelete, zslDeleteByScore and zslDeleteByRank */ /* Internal function used by zslDelete, zslDeleteByScore and zslDeleteByRank */
...@@ -193,7 +194,7 @@ unsigned long zslDeleteRangeByScore(zskiplist *zsl, double min, double max, dict ...@@ -193,7 +194,7 @@ unsigned long zslDeleteRangeByScore(zskiplist *zsl, double min, double max, dict
x = x->level[0].forward; x = x->level[0].forward;
while (x && x->score <= max) { while (x && x->score <= max) {
zskiplistNode *next = x->level[0].forward; zskiplistNode *next = x->level[0].forward;
zslDeleteNode(zsl, x, update); zslDeleteNode(zsl,x,update);
dictDelete(dict,x->obj); dictDelete(dict,x->obj);
zslFreeNode(x); zslFreeNode(x);
removed++; removed++;
...@@ -222,7 +223,7 @@ unsigned long zslDeleteRangeByRank(zskiplist *zsl, unsigned int start, unsigned ...@@ -222,7 +223,7 @@ unsigned long zslDeleteRangeByRank(zskiplist *zsl, unsigned int start, unsigned
x = x->level[0].forward; x = x->level[0].forward;
while (x && traversed <= end) { while (x && traversed <= end) {
zskiplistNode *next = x->level[0].forward; zskiplistNode *next = x->level[0].forward;
zslDeleteNode(zsl, x, update); zslDeleteNode(zsl,x,update);
dictDelete(dict,x->obj); dictDelete(dict,x->obj);
zslFreeNode(x); zslFreeNode(x);
removed++; removed++;
...@@ -299,13 +300,11 @@ zskiplistNode* zslistTypeGetElementByRank(zskiplist *zsl, unsigned long rank) { ...@@ -299,13 +300,11 @@ zskiplistNode* zslistTypeGetElementByRank(zskiplist *zsl, unsigned long rank) {
* Sorted set commands * Sorted set commands
*----------------------------------------------------------------------------*/ *----------------------------------------------------------------------------*/
/* This generic command implements both ZADD and ZINCRBY. /* This generic command implements both ZADD and ZINCRBY. */
* scoreval is the score if the operation is a ZADD (doincrement == 0) or void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double score, int incr) {
* the increment if the operation is a ZINCRBY (doincrement == 1). */
void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, int doincrement) {
robj *zsetobj; robj *zsetobj;
zset *zs; zset *zs;
double *score; zskiplistNode *znode;
zsetobj = lookupKeyWrite(c->db,key); zsetobj = lookupKeyWrite(c->db,key);
if (zsetobj == NULL) { if (zsetobj == NULL) {
...@@ -319,72 +318,73 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i ...@@ -319,72 +318,73 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i
} }
zs = zsetobj->ptr; zs = zsetobj->ptr;
/* Ok now since we implement both ZADD and ZINCRBY here the code /* Since both ZADD and ZINCRBY are implemented here, we need to increment
* needs to handle the two different conditions. It's all about setting * the score first by the current score if ZINCRBY is called. */
* '*score', that is, the new score to set, to the right value. */ if (incr) {
score = zmalloc(sizeof(double));
if (doincrement) {
dictEntry *de;
/* Read the old score. If the element was not present starts from 0 */ /* Read the old score. If the element was not present starts from 0 */
de = dictFind(zs->dict,ele); dictEntry *de = dictFind(zs->dict,ele);
if (de) { if (de != NULL)
double *oldscore = dictGetEntryVal(de); score += *(double*)dictGetEntryVal(de);
*score = *oldscore + scoreval;
} else { if (isnan(score)) {
*score = scoreval;
}
if (isnan(*score)) {
addReplySds(c, addReplySds(c,
sdsnew("-ERR resulting score is not a number (NaN)\r\n")); sdsnew("-ERR resulting score is not a number (NaN)\r\n"));
zfree(score);
/* Note that we don't need to check if the zset may be empty and /* Note that we don't need to check if the zset may be empty and
* should be removed here, as we can only obtain Nan as score if * should be removed here, as we can only obtain Nan as score if
* there was already an element in the sorted set. */ * there was already an element in the sorted set. */
return; return;
} }
} else {
*score = scoreval;
} }
/* What follows is a simple remove and re-insert operation that is common /* We need to remove and re-insert the element when it was already present
* to both ZADD and ZINCRBY... */ * in the dictionary, to update the skiplist. Note that we delay adding a
if (dictAdd(zs->dict,ele,score) == DICT_OK) { * pointer to the score because we want to reference the score in the
/* case 1: New element */ * skiplist node. */
if (dictAdd(zs->dict,ele,NULL) == DICT_OK) {
dictEntry *de;
/* New element */
incrRefCount(ele); /* added to hash */ incrRefCount(ele); /* added to hash */
zslInsert(zs->zsl,*score,ele); znode = zslInsert(zs->zsl,score,ele);
incrRefCount(ele); /* added to skiplist */ incrRefCount(ele); /* added to skiplist */
/* Update the score in the dict entry */
de = dictFind(zs->dict,ele);
redisAssert(de != NULL);
dictGetEntryVal(de) = &znode->score;
touchWatchedKey(c->db,c->argv[1]); touchWatchedKey(c->db,c->argv[1]);
server.dirty++; server.dirty++;
if (doincrement) if (incr)
addReplyDouble(c,*score); addReplyDouble(c,score);
else else
addReply(c,shared.cone); addReply(c,shared.cone);
} else { } else {
dictEntry *de; dictEntry *de;
double *oldscore; robj *curobj;
double *curscore;
int deleted;
/* case 2: Score update operation */ /* Update score */
de = dictFind(zs->dict,ele); de = dictFind(zs->dict,ele);
redisAssert(de != NULL); redisAssert(de != NULL);
oldscore = dictGetEntryVal(de); curobj = dictGetEntryKey(de);
if (*score != *oldscore) { curscore = dictGetEntryVal(de);
int deleted;
/* Remove and insert the element in the skip list with new score */ /* When the score is updated, reuse the existing string object to
deleted = zslDelete(zs->zsl,*oldscore,ele); * prevent extra alloc/dealloc of strings on ZINCRBY. */
if (score != *curscore) {
deleted = zslDelete(zs->zsl,*curscore,curobj);
redisAssert(deleted != 0); redisAssert(deleted != 0);
zslInsert(zs->zsl,*score,ele); znode = zslInsert(zs->zsl,score,curobj);
incrRefCount(ele); incrRefCount(curobj);
/* Update the score in the hash table */
dictReplace(zs->dict,ele,score); /* Update the score in the current dict entry */
dictGetEntryVal(de) = &znode->score;
touchWatchedKey(c->db,c->argv[1]); touchWatchedKey(c->db,c->argv[1]);
server.dirty++; server.dirty++;
} else {
zfree(score);
} }
if (doincrement) if (incr)
addReplyDouble(c,*score); addReplyDouble(c,score);
else else
addReply(c,shared.czero); addReply(c,shared.czero);
} }
...@@ -406,7 +406,7 @@ void zremCommand(redisClient *c) { ...@@ -406,7 +406,7 @@ void zremCommand(redisClient *c) {
robj *zsetobj; robj *zsetobj;
zset *zs; zset *zs;
dictEntry *de; dictEntry *de;
double *oldscore; double curscore;
int deleted; int deleted;
if ((zsetobj = lookupKeyWriteOrReply(c,c->argv[1],shared.czero)) == NULL || if ((zsetobj = lookupKeyWriteOrReply(c,c->argv[1],shared.czero)) == NULL ||
...@@ -419,8 +419,8 @@ void zremCommand(redisClient *c) { ...@@ -419,8 +419,8 @@ void zremCommand(redisClient *c) {
return; return;
} }
/* Delete from the skiplist */ /* Delete from the skiplist */
oldscore = dictGetEntryVal(de); curscore = *(double*)dictGetEntryVal(de);
deleted = zslDelete(zs->zsl,*oldscore,c->argv[2]); deleted = zslDelete(zs->zsl,curscore,c->argv[2]);
redisAssert(deleted != 0); redisAssert(deleted != 0);
/* Delete from the hash table */ /* Delete from the hash table */
...@@ -534,6 +534,7 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { ...@@ -534,6 +534,7 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) {
zsetopsrc *src; zsetopsrc *src;
robj *dstobj; robj *dstobj;
zset *dstzset; zset *dstzset;
zskiplistNode *znode;
dictIterator *di; dictIterator *di;
dictEntry *de; dictEntry *de;
int touched = 0; int touched = 0;
...@@ -642,10 +643,10 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { ...@@ -642,10 +643,10 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) {
zfree(score); zfree(score);
} else { } else {
robj *o = dictGetEntryKey(de); robj *o = dictGetEntryKey(de);
dictAdd(dstzset->dict,o,score); znode = zslInsert(dstzset->zsl,*score,o);
incrRefCount(o); /* added to dictionary */
zslInsert(dstzset->zsl,*score,o);
incrRefCount(o); /* added to skiplist */ incrRefCount(o); /* added to skiplist */
dictAdd(dstzset->dict,o,&znode->score);
incrRefCount(o); /* added to dictionary */
} }
} }
dictReleaseIterator(di); dictReleaseIterator(di);
...@@ -673,10 +674,10 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { ...@@ -673,10 +674,10 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) {
} }
robj *o = dictGetEntryKey(de); robj *o = dictGetEntryKey(de);
dictAdd(dstzset->dict,o,score); znode = zslInsert(dstzset->zsl,*score,o);
incrRefCount(o); /* added to dictionary */
zslInsert(dstzset->zsl,*score,o);
incrRefCount(o); /* added to skiplist */ incrRefCount(o); /* added to skiplist */
dictAdd(dstzset->dict,o,&znode->score);
incrRefCount(o); /* added to dictionary */
} }
dictReleaseIterator(di); dictReleaseIterator(di);
} }
......
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