From 543ede03f2e54b45b22164e17c7f51de9cfc69fa Mon Sep 17 00:00:00 2001 From: antirez Date: Sun, 30 Mar 2014 18:32:17 +0200 Subject: [PATCH] String value unsharing refactored into proper function. All the Redis functions that need to modify the string value of a key in a destructive way (APPEND, SETBIT, SETRANGE, ...) require to make the object unshared (if refcount > 1) and encoded in raw format (if encoding is not already REDIS_ENCODING_RAW). This was cut & pasted many times in multiple places of the code. This commit puts the small logic needed into a function called dbUnshareStringValue(). --- src/bitops.c | 9 +-------- src/db.c | 38 ++++++++++++++++++++++++++++++++++++++ src/hyperloglog.c | 9 +-------- src/object.c | 2 +- src/redis.h | 1 + src/t_string.c | 16 ++-------------- 6 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/bitops.c b/src/bitops.c index 6cf02dc1..28f77243 100644 --- a/src/bitops.c +++ b/src/bitops.c @@ -223,14 +223,7 @@ void setbitCommand(redisClient *c) { dbAdd(c->db,c->argv[1],o); } else { if (checkType(c,o,REDIS_STRING)) return; - - /* Create a copy when the object is shared or encoded. */ - if (o->refcount != 1 || o->encoding != REDIS_ENCODING_RAW) { - robj *decoded = getDecodedObject(o); - o = createRawStringObject(decoded->ptr, sdslen(decoded->ptr)); - decrRefCount(decoded); - dbOverwrite(c->db,c->argv[1],o); - } + o = dbUnshareStringValue(c->db,c->argv[1],o); } /* Grow sds value to the right length if necessary */ diff --git a/src/db.c b/src/db.c index d9dbad2d..f80ef2ac 100644 --- a/src/db.c +++ b/src/db.c @@ -170,6 +170,44 @@ int dbDelete(redisDb *db, robj *key) { } } +/* Prepare the string object stored at 'key' to be modified destructively + * to implement commands like SETBIT or APPEND. + * + * An object is usually ready to be modified unless one of the two conditions + * are true: + * + * 1) The object 'o' is shared (refcount > 1), we don't want to affect + * other users. + * 2) The object encoding is not "RAW". + * + * If the object is found in one of the above conditions (or both) by the + * function, an unshared / not-encoded copy of the string object is stored + * at 'key' in the specified 'db'. Otherwise the object 'o' itself is + * returned. + * + * USAGE: + * + * The object 'o' is what the caller already obtained by looking up 'key' + * in 'db', the usage pattern looks like this: + * + * o = lookupKeyWrite(db,key); + * if (checkType(c,o,REDIS_STRING)) return; + * o = dbUnshareStringValue(db,key,o); + * + * At this point the caller is ready to modify the object, for example + * using an sdscat() call to append some data, or anything else. + */ +robj *dbUnshareStringValue(redisDb *db, robj *key, robj *o) { + redisAssert(o->type == REDIS_STRING); + if (o->refcount != 1 || o->encoding != REDIS_ENCODING_RAW) { + robj *decoded = getDecodedObject(o); + o = createRawStringObject(decoded->ptr, sdslen(decoded->ptr)); + decrRefCount(decoded); + dbOverwrite(db,key,o); + } + return o; +} + long long emptyDb(void(callback)(void*)) { int j; long long removed = 0; diff --git a/src/hyperloglog.c b/src/hyperloglog.c index b94f325f..48d3489a 100644 --- a/src/hyperloglog.c +++ b/src/hyperloglog.c @@ -424,14 +424,7 @@ void hllAddCommand(redisClient *c) { REDIS_HLL_SIZE); return; } - - /* If the object is shared or encoded, we have to make a copy. */ - if (o->refcount != 1 || o->encoding != REDIS_ENCODING_RAW) { - robj *decoded = getDecodedObject(o); - o = createRawStringObject(decoded->ptr, sdslen(decoded->ptr)); - decrRefCount(decoded); - dbOverwrite(c->db,c->argv[1],o); - } + o = dbUnshareStringValue(c->db,c->argv[1],o); } /* Perform the low level ADD operation for every element. */ registers = o->ptr; diff --git a/src/object.c b/src/object.c index c6c1778f..5b9ec5f9 100644 --- a/src/object.c +++ b/src/object.c @@ -660,7 +660,7 @@ unsigned long long estimateObjectIdleTime(robj *o) { } } -/* This is a helper function for the DEBUG command. We need to lookup keys +/* This is a helper function for the OBJECT command. We need to lookup keys * without any modification of LRU or other parameters. */ robj *objectCommandLookup(redisClient *c, robj *key) { dictEntry *de; diff --git a/src/redis.h b/src/redis.h index aaef8f78..5a4c1420 100644 --- a/src/redis.h +++ b/src/redis.h @@ -1252,6 +1252,7 @@ void setKey(redisDb *db, robj *key, robj *val); int dbExists(redisDb *db, robj *key); robj *dbRandomKey(redisDb *db); int dbDelete(redisDb *db, robj *key); +robj *dbUnshareStringValue(redisDb *db, robj *key, robj *o); long long emptyDb(void(callback)(void*)); int selectDb(redisClient *c, int id); void signalModifiedKey(redisDb *db, robj *key); diff --git a/src/t_string.c b/src/t_string.c index 3645ae7c..41e4b3b7 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -215,12 +215,7 @@ void setrangeCommand(redisClient *c) { return; /* Create a copy when the object is shared or encoded. */ - if (o->refcount != 1 || o->encoding != REDIS_ENCODING_RAW) { - robj *decoded = getDecodedObject(o); - o = createRawStringObject(decoded->ptr, sdslen(decoded->ptr)); - decrRefCount(decoded); - dbOverwrite(c->db,c->argv[1],o); - } + o = dbUnshareStringValue(c->db,c->argv[1],o); } if (sdslen(value) > 0) { @@ -433,15 +428,8 @@ void appendCommand(redisClient *c) { if (checkStringLength(c,totlen) != REDIS_OK) return; - /* If the object is shared or encoded, we have to make a copy */ - if (o->refcount != 1 || o->encoding != REDIS_ENCODING_RAW) { - robj *decoded = getDecodedObject(o); - o = createRawStringObject(decoded->ptr, sdslen(decoded->ptr)); - decrRefCount(decoded); - dbOverwrite(c->db,c->argv[1],o); - } - /* Append the value */ + o = dbUnshareStringValue(c->db,c->argv[1],o); o->ptr = sdscatlen(o->ptr,append->ptr,sdslen(append->ptr)); totlen = sdslen(o->ptr); }