fluencelabs/redis
1
0
Fork 0
mirror of https://github.com/fluencelabs/redis synced 2025-03-19 09:00:51 +00:00
Code Issues Projects Releases Wiki Activity

Following @mattsta's friendly review:

Browse Source 
1. memory leak in t_set.c has been fixed
  2. end-of-line spaces has been removed (from all over the place)
  3. for loops have been ordered up to match existing Redis style (less weird)
  4. comments format has been fixed (added * in the beggining of every comment line)
This commit is contained in:
Alon Diamant 2014-12-21 15:08:44 +02:00
parent e3436dd9b8
commit d74a5a0880
2 changed files with 75 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/intset.c
View File

@ -263,20 +263,17 @@ int64_t intsetRandom(intset *is) {
/* How many times bigger should the set length be compared to the requested /* How many times bigger should the set length be compared to the requested
* count of members for us to use the Floyd algorithm instead of * count of members for us to use the Floyd algorithm instead of
the Knuth algorithm */ * the Knuth algorithm */
#define RANDOMMEMBERS_ALGORITHM_SELECTION_RATIO (2) #define RANDOMMEMBERS_ALGORITHM_SELECTION_RATIO (2)
/* Copies 'count' random members from the set into the 'values' array. /* Copies 'count' random members from the set into the 'values' array.
'values' must be an array of int64_t values, of length 'count'. * 'values' must be an array of int64_t values, of length 'count'.
Returns the amount of items returned. If this amount is less than 'count', * Returns the amount of items returned. If this amount is less than 'count',
then the remaining 'values' are left uninitialized. */ * then the remaining 'values' are left uninitialized. */
int intsetRandomMembers(intset *is, int64_t* values, int count) { int intsetRandomMembers(intset *is, int64_t* values, int count) {
/* We don't check that is and values are non-NULL - the caller must /* We don't check that is and values are non-NULL - the caller must
play nice. */ * play nice. */
/* redisAssert(is != NULL);
redisAssert(values != NULL);
redisAssert(count > 0); */
int length = intsetLen(is); int length = intsetLen(is);
@ -286,19 +283,17 @@ int intsetRandomMembers(intset *is, int64_t* values, int count) {
} }
/* Choose between the Knuth shuffle algorithm, O(1) space, O(length) time, /* Choose between the Knuth shuffle algorithm, O(1) space, O(length) time,
and the Floyd algorithm, O(length) space, O(count) time. */ * and the Floyd algorithm, O(length) space, O(count) time. */
if ((RANDOMMEMBERS_ALGORITHM_SELECTION_RATIO * count) > length) { if ((RANDOMMEMBERS_ALGORITHM_SELECTION_RATIO * count) > length) {
/* If the count of members requested is almost the length of the set, /* If the count of members requested is almost the length of the set,
use the Knuth shuffle algorithm, O(1) space, O(length) time. */ * use the Knuth shuffle algorithm, O(1) space, O(length) time. */
/* First, fill the values array with unique random indexes inside /* First, fill the values array with unique random indexes inside
the set. */ * the set. */
int in, im, rn, rm; int in, im, rn, rm;
im = 0; im = 0;
for (in = 0; for (in = 0; in < length && im < count; in++) {
in < length && im < count;
in++) {
rn = length - in; rn = length - in;
rm = count - im; rm = count - im;
@ -310,9 +305,9 @@ int intsetRandomMembers(intset *is, int64_t* values, int count) {
} else { } else {
/* If the length is considerably more than the count of members /* If the length is considerably more than the count of members
requested, use Robert Floyd's algorithm, O(length) space, * requested, use Robert Floyd's algorithm, O(length) space,
O(count) time. * O(count) time.
Based on Jon Bentley's Programming Pearls */ * Based on Jon Bentley's Programming Pearls */
int64_t *is_used = zcalloc(sizeof(int64_t) * length); int64_t *is_used = zcalloc(sizeof(int64_t) * length);
int in, im, r; int in, im, r;
@ -320,16 +315,13 @@ int intsetRandomMembers(intset *is, int64_t* values, int count) {
r = 0; r = 0;
im = 0; im = 0;
for (in = length - count; for (in = length - count; in < length && im < count; in++) {
in < length && im < count;
in++) {
/* Generate a random number r */ /* Generate a random number r */
r = rand() % (in + 1); r = rand() % (in + 1);
/* Do we already have the value in r? */ /* Do we already have the value in r? */
if (is_used[r]) { if (is_used[r]) {
/* Use in instead of the generated number */ /* Use in instead of the generated number */
r = in; r = in;
} }
@ -345,9 +337,7 @@ int intsetRandomMembers(intset *is, int64_t* values, int count) {
/* Replace each random index with the value stored there in the intset */ /* Replace each random index with the value stored there in the intset */
uint8_t encoding = intrev32ifbe(is->encoding); uint8_t encoding = intrev32ifbe(is->encoding);
for (int currentValue = 0; for (int currentValue = 0; currentValue < count; currentValue++) {
currentValue < count;
currentValue++) {
values[currentValue] = values[currentValue] =
_intsetGetEncoded(is, values[currentValue], encoding); _intsetGetEncoded(is, values[currentValue], encoding);
} }

58
src/t_set.c
View File

@ -225,28 +225,30 @@ int setTypeRandomElement(robj *setobj, robj **objele, int64_t *llele) {
* at producing N elements, and the elements are guaranteed to be non * at producing N elements, and the elements are guaranteed to be non
* repeating. * repeating.
*/ */
unsigned long setTypeRandomElements(robj *set, unsigned long count, robj *aux_set) { unsigned long setTypeRandomElements(robj *set, unsigned long count,
robj *aux_set) {
unsigned long set_size; unsigned long set_size;
unsigned long elements_to_return = count; unsigned long elements_to_return = count;
unsigned long elements_copied = 0; unsigned long elements_copied = 0;
unsigned long current_element = 0; unsigned long current_element = 0;
/* Like all setType* functions, we assume good behavior on part of the caller, /* Like all setType* functions, we assume good behavior on part of the
* so no extra parameter checks are made. */ * caller, so no extra parameter checks are made. */
/* If the number of elements in the the set is less than the count /* If the number of elements in the the set is less than the count
requested, just return all of them. */ * requested, just return all of them. */
set_size = setTypeSize(set); set_size = setTypeSize(set);
if (set_size < count) { if (set_size < count) {
elements_to_return = set_size; elements_to_return = set_size;
} }
/* TODO: It is definitely faster adding items to the set by directly handling the Dict /* TODO: It is definitely faster adding items to the set by directly
or intset inside it, avoiding the constant encoding checks inside setTypeAdd(). * handling the Dict or intset inside it, avoiding the constant encoding
However, We don't want to touch the set internals in non setType* functions. * checks inside setTypeAdd(). However, We don't want to touch the set
So, we just call setTypeAdd() multiple times, but this isn't an optimial * internals in non setType* functions. So, we just call setTypeAdd()
solution. * multiple times, but this isn't an optimal solution.
Another option would be to create a bulk-add function: setTypeAddBulk(). */ * Another option would be to create a bulk-add function:
* setTypeAddBulk(). */
if (set->encoding == REDIS_ENCODING_HT) { if (set->encoding == REDIS_ENCODING_HT) {
/* Allocate result array */ /* Allocate result array */
dictEntry **random_elements = dictEntry **random_elements =
@ -258,20 +260,21 @@ unsigned long setTypeRandomElements(robj *set, unsigned long count, robj *aux_se
redisAssert(elements_copied == elements_to_return); redisAssert(elements_copied == elements_to_return);
/* Put them into the set */ /* Put them into the set */
for (current_element = 0; for (current_element = 0; current_element < elements_copied;
current_element < elements_copied;
current_element++) { current_element++) {
/* We get the key and duplicate it, as we know it is a string */ /* We get the key and duplicate it, as we know it is a string */
setTypeAdd(aux_set, setTypeAdd(aux_set,
dupStringObject(dictGetKey(random_elements[current_element]))); dictGetKey(random_elements[current_element]));
} }
zfree(random_elements); zfree(random_elements);
} else if (set->encoding == REDIS_ENCODING_INTSET) { } else if (set->encoding == REDIS_ENCODING_INTSET) {
/* Allocate result array */ /* Allocate result array */
int64_t *random_elements = zmalloc(sizeof(int64_t) * elements_to_return); int64_t *random_elements =
zmalloc(sizeof(int64_t) * elements_to_return);
robj* element_as_str = NULL;
elements_copied = elements_copied =
intsetRandomMembers((intset*) set->ptr, intsetRandomMembers((intset*) set->ptr,
@ -281,14 +284,17 @@ unsigned long setTypeRandomElements(robj *set, unsigned long count, robj *aux_se
redisAssert(elements_copied == elements_to_return); redisAssert(elements_copied == elements_to_return);
/* Put them into the set */ /* Put them into the set */
for (current_element = 0; for (current_element = 0; current_element < elements_copied;
current_element < elements_copied;
current_element++) { current_element++) {
element_as_str = createStringObjectFromLongLong(
random_elements[current_element]);
/* Put the values in the set */ /* Put the values in the set */
setTypeAdd(aux_set, setTypeAdd(aux_set,
createStringObjectFromLongLong( element_as_str);
random_elements[current_element]));
decrRefCount(element_as_str);
} }
zfree(random_elements); zfree(random_elements);
@ -494,19 +500,19 @@ void spopWithCountCommand(redisClient *c) {
} }
/* Make sure a key with the name inputted exists, and that it's type is /* Make sure a key with the name inputted exists, and that it's type is
indeed a set. Otherwise, return nil */ * indeed a set. Otherwise, return nil */
if ((set = lookupKeyReadOrReply(c,c->argv[1],shared.emptymultibulk)) if ((set = lookupKeyReadOrReply(c,c->argv[1],shared.emptymultibulk))
== NULL || checkType(c,set,REDIS_SET)) return; == NULL || checkType(c,set,REDIS_SET)) return;
/* If count is zero, serve an empty multibulk ASAP to avoid special /* If count is zero, serve an empty multibulk ASAP to avoid special
cases later. */ * cases later. */
if (count == 0) { if (count == 0) {
addReply(c,shared.emptymultibulk); addReply(c,shared.emptymultibulk);
return; return;
} }
/* Get the size of the set. It is always > 0, as empty sets get /* Get the size of the set. It is always > 0, as empty sets get
deleted. */ * deleted. */
size = setTypeSize(set); size = setTypeSize(set);
/* Generate an SPOP keyspace notification */ /* Generate an SPOP keyspace notification */
@ -537,13 +543,13 @@ void spopWithCountCommand(redisClient *c) {
* of elements inside the set. */ * of elements inside the set. */
/* We need an auxiliary set. Optimistically, we create a set using an /* We need an auxiliary set. Optimistically, we create a set using an
Intset internally. */ * Intset internally. */
aux = createStringObjectFromLongLong(0); aux = createStringObjectFromLongLong(0);
aux_set = setTypeCreate(aux); aux_set = setTypeCreate(aux);
decrRefCount(aux); decrRefCount(aux);
/* Get the count requested of random elements from the set into our /* Get the count requested of random elements from the set into our
auxiliary set. */ * auxiliary set. */
elements_returned = setTypeRandomElements(set, count, aux_set); elements_returned = setTypeRandomElements(set, count, aux_set);
redisAssert(elements_returned == count); redisAssert(elements_returned == count);
@ -569,7 +575,7 @@ void spopWithCountCommand(redisClient *c) {
} }
else if (element_encoding == REDIS_ENCODING_INTSET) { else if (element_encoding == REDIS_ENCODING_INTSET) {
/* TODO: setTypeRemove() forces us to convert all of the ints /* TODO: setTypeRemove() forces us to convert all of the ints
to string... isn't there a nicer way to do this? */ * to string... isn't there a nicer way to do this? */
objele = createStringObjectFromLongLong(llele); objele = createStringObjectFromLongLong(llele);
addReplyBulk(c, objele); addReplyBulk(c, objele);
@ -590,7 +596,7 @@ void spopWithCountCommand(redisClient *c) {
} }
/* Free the auxiliary set - we need it no more. */ /* Free the auxiliary set - we need it no more. */
freeSetObject(aux_set); decrRefCount(aux_set);
} }
void spopCommand(redisClient *c) { void spopCommand(redisClient *c) {
@ -607,7 +613,7 @@ void spopCommand(redisClient *c) {
} }
/* Make sure a key with the name inputted exists, and that it's type is /* Make sure a key with the name inputted exists, and that it's type is
indeed a set */ * indeed a set */
if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.nullbulk)) == NULL || if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.nullbulk)) == NULL ||
checkType(c,set,REDIS_SET)) return; checkType(c,set,REDIS_SET)) return;