Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add some redis list commands #201

Merged
merged 3 commits into from
Jul 17, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 89 additions & 8 deletions src/data_structure/ziplist/ziplist.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ _ziplist_prev(zipentry_p ze)
return ze - *(ze - 1); /* *(ze - 1) : length of the previous entry */
}

/* do NOT call this function on the last zip entry, use ziplist_prev */
/* do NOT call this function on the last zip entry, use ziplist_next */
static inline zipentry_p
_ziplist_next(zipentry_p ze)
{
Expand Down Expand Up @@ -250,6 +250,42 @@ _ziplist_fromright(const ziplist_p zl, uint32_t idx)
return p - *p + 1;
}

/*
* starting at ze moving from left to right, get entry at offset.
* if overflow, returns end of ziplist tail.
* returns offset of returned entry from ze.
*/
static inline uint32_t
_ziplist_entry_fromleft(zipentry_p *tgt, const ziplist_p zl, const zipentry_p ze, uint32_t offset)
{
ASSERT(zl != NULL);
ASSERT(ze != NULL);
ASSERT(tgt != NULL);
uint32_t ret;
for (*tgt = ze, ret = 0;
offset > 0 && *tgt != zl + ziplist_size(zl);
--offset, ++ret, *tgt += _zipentry_len(*tgt));
return ret;
}

/*
* starting at ze moving from right to left, get entry at offset.
* if underflow, returns ziplist head.
* returns offset of returned entry from ze.
*/
static inline uint32_t
_ziplist_entry_fromright(zipentry_p *tgt, const ziplist_p zl, const zipentry_p ze, uint32_t offset)
{
ASSERT(zl != NULL);
ASSERT(ze != NULL);
ASSERT(tgt != NULL);
uint32_t ret;
for (*tgt = ze, ret = 0;
offset > 0 && *tgt != _ziplist_head(zl);
--offset, ++ret, *tgt -= *(*tgt - 1));
return ret;
}

ziplist_rstatus_e
ziplist_locate(zipentry_p *ze, const ziplist_p zl, int64_t idx)
{
Expand Down Expand Up @@ -342,7 +378,7 @@ ziplist_remove_val(uint32_t *removed, ziplist_p zl, const struct blob *val,
int64_t i = 0;
zipentry_p z;
uint8_t *end;
bool forward = (count > 0);
bool forward = (count >= 0);

if (zl == NULL || val == NULL) {
return ZIPLIST_ERROR;
Expand All @@ -353,12 +389,11 @@ ziplist_remove_val(uint32_t *removed, ziplist_p zl, const struct blob *val,
return ZIPLIST_EINVALID;
}

if (count == 0) {
return ZIPLIST_EINVALID;
}

count = count == 0 ? INT64_MAX : count;

This comment was marked as spam.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, i actually like the original behavior better too. the reason i changes this is that count == 0 for lrem in redis means remove all. i will revert this; perhaps we can move this logic out to protocol if we want to support the redis behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

however, returning error also seems wrong to me. intuitively, it seems like this case should just be a no op

This comment was marked as spam.

atmost = forward ? count : -count;
*removed = 0;
if (removed != NULL) {

This comment was marked as spam.

*removed = 0;
}

/* Encoding one struct blob and follow up with many simple memcmp should be
* faster than decoding each of the zentries being compared.
Expand All @@ -384,7 +419,9 @@ ziplist_remove_val(uint32_t *removed, ziplist_p zl, const struct blob *val,
}

_ziplist_remove(zl, z, _ziplist_next(z), 1);
*removed += 1;
if (removed != NULL) {
++(*removed);
}
}

return ZIPLIST_OK;
Expand Down Expand Up @@ -470,6 +507,50 @@ ziplist_insert(ziplist_p zl, struct blob *val, int64_t idx)
return ZIPLIST_OK;
}

ziplist_rstatus_e
ziplist_trim(ziplist_p zl, int64_t idx, int64_t count)
{
ziplist_rstatus_e status;
uint32_t nentry, zlen;
zipentry_p begin, end;

if (zl == NULL) {
return ZIPLIST_ERROR;
}

if (count > 0) {
/* counting forward from idx, so idx is begin idx */
status = ziplist_locate(&begin, zl, idx);

if (status != ZIPLIST_OK) {
return status;
}

/* get end */
nentry = _ziplist_entry_fromleft(&end, zl, begin, (uint32_t)count);
} else {
/* counting backward from idx, so idx is end idx */
status = ziplist_locate(&end, zl, idx);

if (status != ZIPLIST_OK) {
return status;
}

/* get begin */
nentry = _ziplist_entry_fromright(&begin, zl, end, (uint32_t)-count);
}

ASSERT(end >= begin);

/* clip ziplist to new range, and update header */
zlen = end - begin;
cc_memmove(_ziplist_head(zl), begin, zlen);
ZL_NENTRY(zl) = nentry;
ZL_NEND(zl) = ZIPLIST_HEADER_SIZE + zlen - 1;

return ZIPLIST_OK;
}

ziplist_rstatus_e
ziplist_push(ziplist_p zl, struct blob *val)
{
Expand Down
11 changes: 11 additions & 0 deletions src/data_structure/ziplist/ziplist.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,17 @@ ziplist_rstatus_e ziplist_remove_val(uint32_t *removed, ziplist_p zl, const stru
* CALLER MUST MAKE SURE THERE IS ENOUGH MEMORY!!!
*/
ziplist_rstatus_e ziplist_insert(ziplist_p zl, struct blob *val, int64_t idx);
/*
* trim list down to contain at most count entries, starting at idx. a negative count means
* starting from the end, and gives a list that is non inclusive of idx.
*
* e.g.
* if list contains { 0, 1, 2, 3, 4, 5 }, and we call ziplist_trim(zl, 4, -3),
* the trimmed list will be { 1, 2, 3 }.
*
* if there are fewer than count entries, all entries starting at idx are preserved
*/
ziplist_rstatus_e ziplist_trim(ziplist_p zl, int64_t idx, int64_t count);

This comment was marked as spam.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably haven't thought about this carefully enough either. The most obvious benefit of ziplist_trim as currently implemented right now is it saves on one remove call if we are trimming in the middle of the list. But for other cases, it is essentially the same as remove. Having this API obviously simplifies trim commands, but I can't think of too many other uses for it at the moment.

ziplist_rstatus_e ziplist_push(ziplist_p zl, struct blob *val); /* a shorthand for insert at idx == nentry */
/* remove tail & return, if val is NULL it is equivalent to remove at idx -1 */
ziplist_rstatus_e ziplist_pop(struct blob *val, ziplist_p zl);
Expand Down
2 changes: 1 addition & 1 deletion src/protocol/data/redis/cmd_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* List.delete KEY [VALUE [COUNT]]
*
* trim: trimming a list
* List.trim KEY INDEX [COUNT]
* List.trim KEY INDEX COUNT
*
* len: return number of entries in list
* List.len KEY
Expand Down
2 changes: 1 addition & 1 deletion src/protocol/data/redis/request.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ typedef enum cmd_type {
#undef GET_TYPE

/*
* Note: though redis supports unboudned number of variables in some commands,
* Note: though redis supports unbounded number of variables in some commands,
* implementation cannot operate with performance guarantee when this number
* gets too big. It also introduces uncertainty around resources. Therefore, we
* are limiting it to REQ_NTOKEN minus the # required args. For each command, if
Expand Down
Loading