]> git.saurik.com Git - redis.git/commitdiff
Merge remote branch 'pietern/testverbosity'
authorantirez <antirez@gmail.com>
Fri, 10 Dec 2010 16:24:03 +0000 (17:24 +0100)
committerantirez <antirez@gmail.com>
Fri, 10 Dec 2010 16:24:03 +0000 (17:24 +0100)
src/debug.c
src/redis.h
src/sort.c
src/t_hash.c
src/t_set.c
src/t_string.c
src/valgrind.sup [new file with mode: 0644]
tests/support/server.tcl
tests/test_helper.tcl

index 9e97868ddf35148db63091346acdd03128e33be1..0144c78ca8c338e3eb8485e825ea20cd23dc85db 100644 (file)
@@ -121,7 +121,7 @@ void computeDatasetDigest(unsigned char *final) {
             } else if (o->type == REDIS_SET) {
                 setTypeIterator *si = setTypeInitIterator(o);
                 robj *ele;
-                while((ele = setTypeNext(si)) != NULL) {
+                while((ele = setTypeNextObject(si)) != NULL) {
                     xorObjectDigest(digest,ele);
                     decrRefCount(ele);
                 }
index e012db4c4da9ba009464bcd6cd01e4365f23c0c4..08a377624d41f22a4f6ef9b3e0277e65634862f9 100644 (file)
@@ -813,8 +813,9 @@ int setTypeRemove(robj *subject, robj *value);
 int setTypeIsMember(robj *subject, robj *value);
 setTypeIterator *setTypeInitIterator(robj *subject);
 void setTypeReleaseIterator(setTypeIterator *si);
-robj *setTypeNext(setTypeIterator *si);
-int setTypeRandomElement(robj *setobj, robj **objele, long long *llele);
+int setTypeNext(setTypeIterator *si, robj **objele, int64_t *llele);
+robj *setTypeNextObject(setTypeIterator *si);
+int setTypeRandomElement(robj *setobj, robj **objele, int64_t *llele);
 unsigned long setTypeSize(robj *subject);
 void setTypeConvert(robj *subject, int enc);
 
@@ -822,7 +823,8 @@ void setTypeConvert(robj *subject, int enc);
 void convertToRealHash(robj *o);
 void hashTypeTryConversion(robj *subject, robj **argv, int start, int end);
 void hashTypeTryObjectEncoding(robj *subject, robj **o1, robj **o2);
-robj *hashTypeGet(robj *o, robj *key);
+int hashTypeGet(robj *o, robj *key, robj **objval, unsigned char **v, unsigned int *vlen);
+robj *hashTypeGetObject(robj *o, robj *key);
 int hashTypeExists(robj *o, robj *key);
 int hashTypeSet(robj *o, robj *key, robj *value);
 int hashTypeDelete(robj *o, robj *key);
index 79f7901054c0971e6c52addfbb126bd53afb1854..a44a6d63ba891ca21ea32c2fe881c58b62750dd1 100644 (file)
@@ -76,7 +76,7 @@ robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst) {
         /* Retrieve value from hash by the field name. This operation
          * already increases the refcount of the returned object. */
         initStaticStringObject(fieldobj,((char*)&fieldname)+(sizeof(struct sdshdr)));
-        o = hashTypeGet(o, &fieldobj);
+        o = hashTypeGetObject(o, &fieldobj);
     } else {
         if (o->type != REDIS_STRING) return NULL;
 
@@ -222,7 +222,7 @@ void sortCommand(redisClient *c) {
     } else if (sortval->type == REDIS_SET) {
         setTypeIterator *si = setTypeInitIterator(sortval);
         robj *ele;
-        while((ele = setTypeNext(si)) != NULL) {
+        while((ele = setTypeNextObject(si)) != NULL) {
             vector[j].obj = ele;
             vector[j].u.score = 0;
             vector[j].u.cmpobj = NULL;
index 071b7754a708c2aae9bc0a66982b7f2f7d9c6b1a..21fe9e10970f2419c520f5610b5255f24c18c19b 100644 (file)
@@ -31,27 +31,56 @@ void hashTypeTryObjectEncoding(robj *subject, robj **o1, robj **o2) {
     }
 }
 
-/* Get the value from a hash identified by key. Returns either a string
- * object or NULL if the value cannot be found. The refcount of the object
- * is always increased by 1 when the value was found. */
-robj *hashTypeGet(robj *o, robj *key) {
-    robj *value = NULL;
+/* Get the value from a hash identified by key.
+ *
+ * If the string is found either REDIS_ENCODING_HT or REDIS_ENCODING_ZIPMAP
+ * is returned, and either **objval or **v and *vlen are set accordingly,
+ * so that objects in hash tables are returend as objects and pointers
+ * inside a zipmap are returned as such.
+ *
+ * If the object was not found -1 is returned.
+ *
+ * This function is copy on write friendly as there is no incr/decr
+ * of refcount needed if objects are accessed just for reading operations. */
+int hashTypeGet(robj *o, robj *key, robj **objval, unsigned char **v,
+                unsigned int *vlen)
+{
     if (o->encoding == REDIS_ENCODING_ZIPMAP) {
-        unsigned char *v;
-        unsigned int vlen;
+        int found;
+
         key = getDecodedObject(key);
-        if (zipmapGet(o->ptr,key->ptr,sdslen(key->ptr),&v,&vlen)) {
-            value = createStringObject((char*)v,vlen);
-        }
+        found = zipmapGet(o->ptr,key->ptr,sdslen(key->ptr),v,vlen);
         decrRefCount(key);
+        if (!found) return -1;
     } else {
         dictEntry *de = dictFind(o->ptr,key);
-        if (de != NULL) {
-            value = dictGetEntryVal(de);
-            incrRefCount(value);
-        }
+        if (de == NULL) return -1;
+        *objval = dictGetEntryVal(de);
+    }
+    return o->encoding;
+}
+
+/* Higher level function of hashTypeGet() that always returns a Redis
+ * object (either new or with refcount incremented), so that the caller
+ * can retain a reference or call decrRefCount after the usage.
+ *
+ * The lower level function can prevent copy on write so it is
+ * the preferred way of doing read operations. */
+robj *hashTypeGetObject(robj *o, robj *key) {
+    robj *objval;
+    unsigned char *v;
+    unsigned int vlen;
+
+    int encoding = hashTypeGet(o,key,&objval,&v,&vlen);
+    switch(encoding) {
+        case REDIS_ENCODING_HT:
+            incrRefCount(objval);
+            return objval;
+        case REDIS_ENCODING_ZIPMAP:
+            objval = createStringObject((char*)v,vlen);
+            return objval;
+        default: return NULL;
     }
-    return value;
 }
 
 /* Test if the key exists in the given hash. Returns 1 if the key
@@ -270,7 +299,7 @@ void hincrbyCommand(redisClient *c) {
 
     if (getLongLongFromObjectOrReply(c,c->argv[3],&incr,NULL) != REDIS_OK) return;
     if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return;
-    if ((current = hashTypeGet(o,c->argv[2])) != NULL) {
+    if ((current = hashTypeGetObject(o,c->argv[2])) != NULL) {
         if (getLongLongFromObjectOrReply(c,current,&value,
             "hash value is not an integer") != REDIS_OK) {
             decrRefCount(current);
@@ -293,20 +322,29 @@ void hincrbyCommand(redisClient *c) {
 
 void hgetCommand(redisClient *c) {
     robj *o, *value;
+    unsigned char *v;
+    unsigned int vlen;
+    int encoding;
+
     if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.nullbulk)) == NULL ||
         checkType(c,o,REDIS_HASH)) return;
 
-    if ((value = hashTypeGet(o,c->argv[2])) != NULL) {
-        addReplyBulk(c,value);
-        decrRefCount(value);
+    if ((encoding = hashTypeGet(o,c->argv[2],&value,&v,&vlen)) != -1) {
+        if (encoding == REDIS_ENCODING_HT)
+            addReplyBulk(c,value);
+        else
+            addReplyBulkCBuffer(c,v,vlen);
     } else {
         addReply(c,shared.nullbulk);
     }
 }
 
 void hmgetCommand(redisClient *c) {
-    int i;
+    int i, encoding;
     robj *o, *value;
+    unsigned char *v;
+    unsigned int vlen;
+
     o = lookupKeyRead(c->db,c->argv[1]);
     if (o != NULL && o->type != REDIS_HASH) {
         addReply(c,shared.wrongtypeerr);
@@ -318,9 +356,12 @@ void hmgetCommand(redisClient *c) {
      * an empty hash. The reply should then be a series of NULLs. */
     addReplyMultiBulkLen(c,c->argc-2);
     for (i = 2; i < c->argc; i++) {
-        if (o != NULL && (value = hashTypeGet(o,c->argv[i])) != NULL) {
-            addReplyBulk(c,value);
-            decrRefCount(value);
+        if (o != NULL &&
+            (encoding = hashTypeGet(o,c->argv[i],&value,&v,&vlen)) != -1) {
+            if (encoding == REDIS_ENCODING_HT)
+                addReplyBulk(c,value);
+            else
+                addReplyBulkCBuffer(c,v,vlen);
         } else {
             addReply(c,shared.nullbulk);
         }
index e15952c05c6c9c0979dc878113fc325bdd14d6ba..0b4128adf4c804053b9e237b0a7a9b9f354e8f1c 100644 (file)
@@ -101,39 +101,68 @@ void setTypeReleaseIterator(setTypeIterator *si) {
 }
 
 /* Move to the next entry in the set. Returns the object at the current
- * position, or NULL when the end is reached. This object will have its
- * refcount incremented, so the caller needs to take care of this. */
-robj *setTypeNext(setTypeIterator *si) {
-    robj *ret = NULL;
+ * position.
+ *
+ * Since set elements can be internally be stored as redis objects or
+ * simple arrays of integers, setTypeNext returns the encoding of the
+ * set object you are iterating, and will populate the appropriate pointer
+ * (eobj) or (llobj) accordingly.
+ *
+ * When there are no longer elements -1 is returned.
+ * Returned objects ref count is not incremented, so this function is
+ * copy on write friendly. */
+int setTypeNext(setTypeIterator *si, robj **objele, int64_t *llele) {
     if (si->encoding == REDIS_ENCODING_HT) {
         dictEntry *de = dictNext(si->di);
-        if (de != NULL) {
-            ret = dictGetEntryKey(de);
-            incrRefCount(ret);
-        }
+        if (de == NULL) return -1;
+        *objele = dictGetEntryKey(de);
     } else if (si->encoding == REDIS_ENCODING_INTSET) {
-        int64_t llval;
-        if (intsetGet(si->subject->ptr,si->ii++,&llval))
-            ret = createStringObjectFromLongLong(llval);
+        if (!intsetGet(si->subject->ptr,si->ii++,llele))
+            return -1;
     }
-    return ret;
+    return si->encoding;
 }
 
+/* The not copy on write friendly version but easy to use version
+ * of setTypeNext() is setTypeNextObject(), returning new objects
+ * or incrementing the ref count of returned objects. So if you don't
+ * retain a pointer to this object you should call decrRefCount() against it.
+ *
+ * This function is the way to go for write operations where COW is not
+ * an issue as the result will be anyway of incrementing the ref count. */
+robj *setTypeNextObject(setTypeIterator *si) {
+    int64_t intele;
+    robj *objele;
+    int encoding;
+
+    encoding = setTypeNext(si,&objele,&intele);
+    switch(encoding) {
+        case -1:    return NULL;
+        case REDIS_ENCODING_INTSET:
+            return createStringObjectFromLongLong(intele);
+        case REDIS_ENCODING_HT:
+            incrRefCount(objele);
+            return objele;
+        default:
+            redisPanic("Unsupported encoding");
+    }
+    return NULL; /* just to suppress warnings */
+}
 
 /* Return random element from a non empty set.
- * The returned element can be a long long value if the set is encoded
+ * The returned element can be a int64_t value if the set is encoded
  * as an "intset" blob of integers, or a redis object if the set
  * is a regular set.
  *
  * The caller provides both pointers to be populated with the right
  * object. The return value of the function is the object->encoding
  * field of the object and is used by the caller to check if the
- * long long pointer or the redis object pointere was populated.
+ * int64_t pointer or the redis object pointere was populated.
  *
  * When an object is returned (the set was a real set) the ref count
  * of the object is not incremented so this function can be considered
- * copy-on-write friendly. */
-int setTypeRandomElement(robj *setobj, robj **objele, long long *llele) {
+ * copy on write friendly. */
+int setTypeRandomElement(robj *setobj, robj **objele, int64_t *llele) {
     if (setobj->encoding == REDIS_ENCODING_HT) {
         dictEntry *de = dictGetRandomKey(setobj->ptr);
         *objele = dictGetEntryKey(de);
@@ -158,25 +187,30 @@ unsigned long setTypeSize(robj *subject) {
 /* Convert the set to specified encoding. The resulting dict (when converting
  * to a hashtable) is presized to hold the number of elements in the original
  * set. */
-void setTypeConvert(robj *subject, int enc) {
+void setTypeConvert(robj *setobj, int enc) {
     setTypeIterator *si;
-    robj *element;
-    redisAssert(subject->type == REDIS_SET);
+    redisAssert(setobj->type == REDIS_SET &&
+                setobj->encoding == REDIS_ENCODING_INTSET);
 
     if (enc == REDIS_ENCODING_HT) {
+        int64_t intele;
         dict *d = dictCreate(&setDictType,NULL);
+        robj *element;
+
         /* Presize the dict to avoid rehashing */
-        dictExpand(d,intsetLen(subject->ptr));
+        dictExpand(d,intsetLen(setobj->ptr));
 
-        /* setTypeGet returns a robj with incremented refcount */
-        si = setTypeInitIterator(subject);
-        while ((element = setTypeNext(si)) != NULL)
+        /* To add the elements we extract integers and create redis objects */
+        si = setTypeInitIterator(setobj);
+        while (setTypeNext(si,NULL,&intele) != -1) {
+            element = createStringObjectFromLongLong(intele);
             redisAssert(dictAdd(d,element,NULL) == DICT_OK);
+        }
         setTypeReleaseIterator(si);
 
-        subject->encoding = REDIS_ENCODING_HT;
-        zfree(subject->ptr);
-        subject->ptr = d;
+        setobj->encoding = REDIS_ENCODING_HT;
+        zfree(setobj->ptr);
+        setobj->ptr = d;
     } else {
         redisPanic("Unsupported set conversion");
     }
@@ -292,7 +326,7 @@ void scardCommand(redisClient *c) {
 
 void spopCommand(redisClient *c) {
     robj *set, *ele;
-    long long llele;
+    int64_t llele;
     int encoding;
 
     if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.nullbulk)) == NULL ||
@@ -313,7 +347,7 @@ void spopCommand(redisClient *c) {
 
 void srandmemberCommand(redisClient *c) {
     robj *set, *ele;
-    long long llele;
+    int64_t llele;
     int encoding;
 
     if ((set = lookupKeyReadOrReply(c,c->argv[1],shared.nullbulk)) == NULL ||
@@ -334,9 +368,11 @@ int qsortCompareSetsByCardinality(const void *s1, const void *s2) {
 void sinterGenericCommand(redisClient *c, robj **setkeys, unsigned long setnum, robj *dstkey) {
     robj **sets = zmalloc(sizeof(robj*)*setnum);
     setTypeIterator *si;
-    robj *ele, *dstset = NULL;
+    robj *eleobj, *dstset = NULL;
+    int64_t intobj;
     void *replylen = NULL;
     unsigned long j, cardinality = 0;
+    int encoding;
 
     for (j = 0; j < setnum; j++) {
         robj *setobj = dstkey ?
@@ -382,20 +418,60 @@ void sinterGenericCommand(redisClient *c, robj **setkeys, unsigned long setnum,
      * the element against all the other sets, if at least one set does
      * not include the element it is discarded */
     si = setTypeInitIterator(sets[0]);
-    while((ele = setTypeNext(si)) != NULL) {
-        for (j = 1; j < setnum; j++)
-            if (!setTypeIsMember(sets[j],ele)) break;
+    while((encoding = setTypeNext(si,&eleobj,&intobj)) != -1) {
+        for (j = 1; j < setnum; j++) {
+            if (encoding == REDIS_ENCODING_INTSET) {
+                /* intset with intset is simple... and fast */
+                if (sets[j]->encoding == REDIS_ENCODING_INTSET &&
+                    !intsetFind((intset*)sets[j]->ptr,intobj))
+                {
+                    break;
+                /* in order to compare an integer with an object we
+                 * have to use the generic function, creating an object
+                 * for this */
+                } else if (sets[j]->encoding == REDIS_ENCODING_HT) {
+                    eleobj = createStringObjectFromLongLong(intobj);
+                    if (!setTypeIsMember(sets[j],eleobj)) {
+                        decrRefCount(eleobj);
+                        break;
+                    }
+                    decrRefCount(eleobj);
+                }
+            } else if (encoding == REDIS_ENCODING_HT) {
+                /* Optimization... if the source object is integer
+                 * encoded AND the target set is an intset, we can get
+                 * a much faster path. */
+                if (eleobj->encoding == REDIS_ENCODING_INT &&
+                    sets[j]->encoding == REDIS_ENCODING_INTSET &&
+                    !intsetFind((intset*)sets[j]->ptr,(long)eleobj->ptr))
+                {
+                    break;
+                /* else... object to object check is easy as we use the
+                 * type agnostic API here. */
+                } else if (!setTypeIsMember(sets[j],eleobj)) {
+                    break;
+                }
+            }
+        }
 
         /* Only take action when all sets contain the member */
         if (j == setnum) {
             if (!dstkey) {
-                addReplyBulk(c,ele);
+                if (encoding == REDIS_ENCODING_HT)
+                    addReplyBulk(c,eleobj);
+                else
+                    addReplyBulkLongLong(c,intobj);
                 cardinality++;
             } else {
-                setTypeAdd(dstset,ele);
+                if (encoding == REDIS_ENCODING_INTSET) {
+                    eleobj = createStringObjectFromLongLong(intobj);
+                    setTypeAdd(dstset,eleobj);
+                    decrRefCount(eleobj);
+                } else {
+                    setTypeAdd(dstset,eleobj);
+                }
             }
         }
-        decrRefCount(ele);
     }
     setTypeReleaseIterator(si);
 
@@ -463,7 +539,7 @@ void sunionDiffGenericCommand(redisClient *c, robj **setkeys, int setnum, robj *
         if (!sets[j]) continue; /* non existing keys are like empty sets */
 
         si = setTypeInitIterator(sets[j]);
-        while((ele = setTypeNext(si)) != NULL) {
+        while((ele = setTypeNextObject(si)) != NULL) {
             if (op == REDIS_OP_UNION || j == 0) {
                 if (setTypeAdd(dstset,ele)) {
                     cardinality++;
@@ -485,7 +561,7 @@ void sunionDiffGenericCommand(redisClient *c, robj **setkeys, int setnum, robj *
     if (!dstkey) {
         addReplyMultiBulkLen(c,cardinality);
         si = setTypeInitIterator(dstset);
-        while((ele = setTypeNext(si)) != NULL) {
+        while((ele = setTypeNextObject(si)) != NULL) {
             addReplyBulk(c,ele);
             decrRefCount(ele);
         }
index 39ee506d5e9586851ad8bf603859ca2c0b2d15b7..bb6b4eceb5628dbf258e2452ab7e28a88851811c 100644 (file)
@@ -144,8 +144,24 @@ void incrDecrCommand(redisClient *c, long long incr) {
 
     o = lookupKeyWrite(c->db,c->argv[1]);
     if (o != NULL && checkType(c,o,REDIS_STRING)) return;
-    if (getLongLongFromObjectOrReply(c,o,&value,NULL) != REDIS_OK) return;
 
+    /* Fast path if the object is integer encoded and is not shared. */
+    if (o && o->refcount == 1 && o->encoding == REDIS_ENCODING_INT) {
+        long long newval = ((long)o->ptr) + incr;
+
+        if (newval < 0 && newval >= REDIS_SHARED_INTEGERS &&
+            newval >= LONG_MIN && newval <= LONG_MAX) {
+            o->ptr = (void*) (long) newval;
+            touchWatchedKey(c->db,c->argv[1]);
+            server.dirty++;
+            addReplyLongLong(c,newval);
+            return;
+        }
+        /* ... else take the usual safe path */
+    }
+
+    /* Otherwise we create a new object and replace the old one. */
+    if (getLongLongFromObjectOrReply(c,o,&value,NULL) != REDIS_OK) return;
     value += incr;
     o = createStringObjectFromLongLong(value);
     dbReplace(c->db,c->argv[1],o);
diff --git a/src/valgrind.sup b/src/valgrind.sup
new file mode 100644 (file)
index 0000000..7ba7575
--- /dev/null
@@ -0,0 +1,5 @@
+{
+   <lzf_unitialized_hash_table>
+   Memcheck:Cond
+   fun:lzf_compress
+}
index 39038bd699cfadb60b273c8d1c386254d8a5e699..4f48d22dcafbd023acde952340890bc262a7ee10 100644 (file)
@@ -175,7 +175,7 @@ proc start_server {options {code undefined}} {
     set stderr [format "%s/%s" [dict get $config "dir"] "stderr"]
 
     if {$::valgrind} {
-        exec valgrind src/redis-server $config_file > $stdout 2> $stderr &
+        exec valgrind --suppressions=src/valgrind.sup src/redis-server $config_file > $stdout 2> $stderr &
     } else {
         exec src/redis-server $config_file > $stdout 2> $stderr &
     }
index 1852fa7b5d7485473fab2f851def5e0f3148e723..2b7a8957772dbd1e57b4f73bfa362fe8392f1205 100644 (file)
@@ -189,6 +189,8 @@ for {set j 0} {$j < [llength $argv]} {incr j} {
             }
         }
         incr j
+    } elseif {$opt eq {--valgrind}} {
+        set ::valgrind 1
     } elseif {$opt eq {--file}} {
         set ::file $arg
         incr j