From 8eaf5168378bbd35640e49dde5778b9aeae055e4 Mon Sep 17 00:00:00 2001 From: Sun He Date: Thu, 14 May 2015 22:26:06 +0800 Subject: [PATCH 1/3] skiplist: add iter APIs --- src/skiplist.c | 69 +++++++++++++++++++++++++++++++++++++++++++++----- src/skiplist.h | 14 ++++++++++ 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/skiplist.c b/src/skiplist.c index 0234896..11c5d67 100644 --- a/src/skiplist.c +++ b/src/skiplist.c @@ -265,6 +265,64 @@ unsigned long skiplistLength(skiplist *sl) { return sl->length; } +/* Returns a skiplist iterator 'iter'. After the initialization every + * call to listNext() will return the next element of the list. + * + * This function can't fail. */ +skiplistIter *skiplistGetIterator(skiplist *skiplist, int direction) { + skiplistIter *iter; + + if ((iter = zmalloc(sizeof(*iter))) == NULL) return NULL; + if (direction == SL_START_HEAD) + iter->next = skiplist->header->level[0].forward; + else + iter->next = skiplist->tail; + iter->direction = direction; + return iter; +} + +/* Release the iterator memory */ +void skiplistReleaseIterator(skiplistIter *iter) { + zfree(iter); +} + +/* Return the next element of an iterator. + * It's valid to remove the currently returned element using + * skiplistDelete(), but not to remove other elements. + * + * The function returns a pointer to the next element of the skiplist, + * or NULL if there are no more elements, so the classical usage patter + * is: + * + * iter = skiplistGetIterator(list,); + * while ((node = skiplistNext(iter)) != NULL) { + * doSomethingWith(skiplistNodeValue(node)); + * } + * + * */ +skiplistNode *skiplistNext(skiplistIter *iter) { + skiplistNode *current = iter->next; + + if (current != NULL) { + if (iter->direction == SL_START_HEAD) + iter->next = current->level[0].forward; + else + iter->next = current->backward; + } + return current; +} + +/* Create an iterator in the list private iterator structure */ +void skiplistRewind(skiplist *skiplist, skiplistIter *sli) { + sli->next = skiplist->header->level[0].forward; + sli->direction = SL_START_HEAD; +} + +void skiplistRewindTail(skiplist *skiplist, skiplistIter *sli) { + sli->next = skiplist->tail; + sli->direction = SL_START_TAIL; +} + #ifdef TEST_MAIN #include #include @@ -288,12 +346,11 @@ int main(void) { /* The following should fail. */ printf("\nInsert %s again: %p\n\n", words[2], skiplistInsert(sl,words[2])); - skiplistNode *x; - x = sl->header; - x = x->level[0].forward; - while(x) { - printf("%s\n", x->obj); - x = x->level[0].forward; + skiplistIter skiplist_iter; + skiplistNode *node; + skiplistRewind(sl, &skiplist_iter); + while ((node = skiplistNext(&skiplist_iter)) != NULL) { + printf("%s\n", node->obj); } printf("Searching for 'hello': %p\n", skiplistFind(sl,"hello")); diff --git a/src/skiplist.h b/src/skiplist.h index cba3415..c795dd8 100644 --- a/src/skiplist.h +++ b/src/skiplist.h @@ -51,6 +51,11 @@ typedef struct skiplist { int level; } skiplist; +typedef struct skiplistIter { + skiplistNode *next; + int direction; +} skiplistIter; + skiplist *skiplistCreate(int (*compare)(const void *, const void *)); void skiplistFree(skiplist *sl); skiplistNode *skiplistInsert(skiplist *sl, void *obj); @@ -59,5 +64,14 @@ void *skiplistFind(skiplist *sl, void *obj); void *skiplistPopHead(skiplist *sl); void *skiplistPopTail(skiplist *sl); unsigned long skiplistLength(skiplist *sl); +skiplistIter *skiplistGetIterator(skiplist *skiplist, int direction); +void skiplistReleaseIterator(skiplistIter *iter); +skiplistNode *skiplistNext(skiplistIter *iter); +void skiplistRewind(skiplist *skiplist, skiplistIter *sli); +void skiplistRewindTail(skiplist *skiplist, skiplistIter *sli); + +/* Directions for iterators */ +#define SL_START_HEAD 0 +#define SL_START_TAIL 1 #endif From 9064a481a03b9ae95e204bf32706223bbd6565ad Mon Sep 17 00:00:00 2001 From: Sun He Date: Thu, 14 May 2015 22:49:35 +0800 Subject: [PATCH 2/3] use skiplistIter --- src/job.c | 12 ++++++------ src/queue.c | 14 +++++++++----- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/job.c b/src/job.c index 7a88bf2..a342a62 100644 --- a/src/job.c +++ b/src/job.c @@ -430,7 +430,7 @@ int processJobs(struct aeEventLoop *eventLoop, long long id, void *clientData) { int period = 100; /* 100 ms default period. */ int max = 10000; /* 10k jobs * 1000 milliseconds = 10M jobs/sec max. */ mstime_t now = mstime(), latency; - skiplistNode *current, *next; + skiplistNode *current; DISQUE_NOTUSED(eventLoop); DISQUE_NOTUSED(id); DISQUE_NOTUSED(clientData); @@ -449,8 +449,9 @@ int processJobs(struct aeEventLoop *eventLoop, long long id, void *clientData) { latencyStartMonitor(latency); server.mstime = now; /* Update it since it's used by processJob(). */ - current = server.awakeme->header->level[0].forward; - while(current && max--) { + skiplistIter iter; + skiplistRewind(server.awakeme, &iter); + while((current = skiplistNext(&iter)) != NULL && max--) { job *j = current->obj; #ifdef DEBUG_SCHEDULER @@ -464,16 +465,15 @@ int processJobs(struct aeEventLoop *eventLoop, long long id, void *clientData) { #endif if (j->awakeme > now) break; - next = current->level[0].forward; processJob(j); - current = next; } /* Try to block between 1 and 100 millseconds depending on how near * in time is the next async event to process. Note that because of * received commands or change in state jobs state may be modified so * we set a max time of 100 milliseconds to wakeup anyway. */ - current = server.awakeme->header->level[0].forward; + skiplistRewind(server.awakeme, &iter); + current = skiplistNext(&iter); if (current) { job *j = current->obj; period = server.mstime-j->awakeme; diff --git a/src/queue.c b/src/queue.c index 5827135..9452ade 100644 --- a/src/queue.c +++ b/src/queue.c @@ -767,11 +767,18 @@ void qpeekCommand(client *c) { newjobs = 1; } + skiplistIter iter; skiplistNode *sn = NULL; queue *q = lookupQueue(c->argv[1]); if (q != NULL) - sn = newjobs ? q->sl->tail : q->sl->header->level[0].forward; + { + if (newjobs) + skiplistRewindTail(server.awakeme, &iter); + else + skiplistRewind(server.awakeme, &iter); + sn = skiplistNext(&iter); + } if (sn == NULL) { addReply(c,shared.emptymultibulk); @@ -785,10 +792,7 @@ void qpeekCommand(client *c) { addReplyBulkCBuffer(c,j->id,JOB_ID_LEN); addReplyBulkCBuffer(c,j->body,sdslen(j->body)); returned++; - if (newjobs) - sn = sn->backward; - else - sn = sn->level[0].forward; + sn = skiplistNext(&iter); } setDeferredMultiBulkLength(c,deflen,returned); } From b2ca89da01743da2313d7ef6d42040900cfa8c5b Mon Sep 17 00:00:00 2001 From: Sun He Date: Sat, 16 May 2015 22:35:13 +0800 Subject: [PATCH 3/3] skiplistRewind: change funcs to macro and inline func Follow antirez's advice in #67 skiplistRewind/skiplistRewindTail changed to macros, skiplistNext changed to an inline func, Iterator should be guaranteed stack allocated and no cleanup need. --- src/skiplist.c | 58 -------------------------------------------------- src/skiplist.h | 27 ++++++++++++++++++----- 2 files changed, 22 insertions(+), 63 deletions(-) diff --git a/src/skiplist.c b/src/skiplist.c index 11c5d67..06f301d 100644 --- a/src/skiplist.c +++ b/src/skiplist.c @@ -265,64 +265,6 @@ unsigned long skiplistLength(skiplist *sl) { return sl->length; } -/* Returns a skiplist iterator 'iter'. After the initialization every - * call to listNext() will return the next element of the list. - * - * This function can't fail. */ -skiplistIter *skiplistGetIterator(skiplist *skiplist, int direction) { - skiplistIter *iter; - - if ((iter = zmalloc(sizeof(*iter))) == NULL) return NULL; - if (direction == SL_START_HEAD) - iter->next = skiplist->header->level[0].forward; - else - iter->next = skiplist->tail; - iter->direction = direction; - return iter; -} - -/* Release the iterator memory */ -void skiplistReleaseIterator(skiplistIter *iter) { - zfree(iter); -} - -/* Return the next element of an iterator. - * It's valid to remove the currently returned element using - * skiplistDelete(), but not to remove other elements. - * - * The function returns a pointer to the next element of the skiplist, - * or NULL if there are no more elements, so the classical usage patter - * is: - * - * iter = skiplistGetIterator(list,); - * while ((node = skiplistNext(iter)) != NULL) { - * doSomethingWith(skiplistNodeValue(node)); - * } - * - * */ -skiplistNode *skiplistNext(skiplistIter *iter) { - skiplistNode *current = iter->next; - - if (current != NULL) { - if (iter->direction == SL_START_HEAD) - iter->next = current->level[0].forward; - else - iter->next = current->backward; - } - return current; -} - -/* Create an iterator in the list private iterator structure */ -void skiplistRewind(skiplist *skiplist, skiplistIter *sli) { - sli->next = skiplist->header->level[0].forward; - sli->direction = SL_START_HEAD; -} - -void skiplistRewindTail(skiplist *skiplist, skiplistIter *sli) { - sli->next = skiplist->tail; - sli->direction = SL_START_TAIL; -} - #ifdef TEST_MAIN #include #include diff --git a/src/skiplist.h b/src/skiplist.h index c795dd8..b68e028 100644 --- a/src/skiplist.h +++ b/src/skiplist.h @@ -64,14 +64,31 @@ void *skiplistFind(skiplist *sl, void *obj); void *skiplistPopHead(skiplist *sl); void *skiplistPopTail(skiplist *sl); unsigned long skiplistLength(skiplist *sl); -skiplistIter *skiplistGetIterator(skiplist *skiplist, int direction); -void skiplistReleaseIterator(skiplistIter *iter); -skiplistNode *skiplistNext(skiplistIter *iter); -void skiplistRewind(skiplist *skiplist, skiplistIter *sli); -void skiplistRewindTail(skiplist *skiplist, skiplistIter *sli); /* Directions for iterators */ #define SL_START_HEAD 0 #define SL_START_TAIL 1 +/* Create an iterator in the skiplist private iterator structure + * skiplistIter should be a pointer point to the struct. + * */ +#define skiplistRewind(skiplist, skiplistIter) do { \ + (skiplistIter)->next = (skiplist)->header->level[0].forward; \ + (skiplistIter)->direction = SL_START_HEAD; \ +} while(0) + +#define skiplistRewindTail(skiplist, skiplistIter) do { \ + (skiplistIter)->next = (skiplist)->tail; \ + (skiplistIter)->direction = SL_START_TAIL; \ +} while(0) + +inline skiplistNode *skiplistNext(skiplistIter *iter) { + skiplistNode *current = iter->next; + if (current != NULL) { + iter->next = iter->direction == SL_START_HEAD ? + current->level[0].forward : current->backward; + } + return current; +} + #endif