From acb32d472da72a36dc73eca5fef1df71cffdc328 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Viktor=20S=C3=B6derqvist?= <viktor.soderqvist@est.tech>
Date: Fri, 12 Feb 2021 18:03:10 +0100
Subject: [PATCH] Add ziplistReplace, in-place optimized for elements of same
size
Avoids memmove and reallocs when replacing a ziplist element of the
same encoded size as the new value.
Affects HSET, HINRBY, HINCRBYFLOAT (via hashTypeSet) and LSET (via
quicklistReplaceAtIndex).
---
src/quicklist.c | 3 +--
src/t_hash.c | 7 ++---
src/ziplist.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
src/ziplist.h | 1 +
4 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/src/quicklist.c b/src/quicklist.c
index b2a406004..c8517414c 100644
--- a/src/quicklist.c
+++ b/src/quicklist.c
@@ -680,8 +680,7 @@ int quicklistReplaceAtIndex(quicklist *quicklist, long index, void *data,
quicklistEntry entry;
if (likely(quicklistIndex(quicklist, index, &entry))) {
/* quicklistIndex provides an uncompressed node */
- entry.node->zl = ziplistDelete(entry.node->zl, &entry.zi);
- entry.node->zl = ziplistInsert(entry.node->zl, entry.zi, data, sz);
+ entry.node->zl = ziplistReplace(entry.node->zl, entry.zi, data, sz);
quicklistNodeUpdateSz(entry.node);
quicklistCompress(quicklist, entry.node);
return 1;
diff --git a/src/t_hash.c b/src/t_hash.c
index cae350566..15b57c1d7 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -215,11 +215,8 @@ int hashTypeSet(robj *o, sds field, sds value, int flags) {
serverAssert(vptr != NULL);
update = 1;
- /* Delete value */
- zl = ziplistDelete(zl, &vptr);
-
- /* Insert new value */
- zl = ziplistInsert(zl, vptr, (unsigned char*)value,
+ /* Replace value */
+ zl = ziplistReplace(zl, vptr, (unsigned char*)value,
sdslen(value));
}
}
diff --git a/src/ziplist.c b/src/ziplist.c
index 35a0f283a..b66f97ef8 100644
--- a/src/ziplist.c
+++ b/src/ziplist.c
@@ -1265,6 +1265,42 @@ unsigned char *ziplistDeleteRange(unsigned char *zl, int index, unsigned int num
return (p == NULL) ? zl : __ziplistDelete(zl,p,num);
}
+/* Replaces the entry at p. This is equivalent to a delete and an insert,
+ * but avoids some overhead when replacing a value of the same size. */
+unsigned char *ziplistReplace(unsigned char *zl, unsigned char *p, unsigned char *s, unsigned int slen) {
+
+ /* get metadata of the current entry */
+ zlentry entry;
+ zipEntry(p, &entry);
+
+ /* compute length of entry to store, excluding prevlen */
+ unsigned int reqlen;
+ unsigned char encoding = 0;
+ long long value = 123456789; /* initialized to avoid warning. */
+ if (zipTryEncoding(s,slen,&value,&encoding)) {
+ reqlen = zipIntSize(encoding); /* encoding is set */
+ } else {
+ reqlen = slen; /* encoding == 0 */
+ }
+ reqlen += zipStoreEntryEncoding(NULL,encoding,slen);
+
+ if (reqlen == entry.lensize + entry.len) {
+ /* Simply overwrite the element. */
+ p += entry.prevrawlensize;
+ p += zipStoreEntryEncoding(p,encoding,slen);
+ if (ZIP_IS_STR(encoding)) {
+ memcpy(p,s,slen);
+ } else {
+ zipSaveInteger(p,value,encoding);
+ }
+ } else {
+ /* Fallback. */
+ zl = ziplistDelete(zl,&p);
+ zl = ziplistInsert(zl,p,s,slen);
+ }
+ return zl;
+}
+
/* Compare entry pointer to by 'p' with 'sstr' of length 'slen'. */
/* Return 1 if equal. */
unsigned int ziplistCompare(unsigned char *p, unsigned char *sstr, unsigned int slen) {
@@ -2070,6 +2106,41 @@ int ziplistTest(int argc, char **argv) {
zfree(zl);
}
+ printf("Replace with same size:\n");
+ {
+ zl = createList(); /* "hello", "foo", "quux", "1024" */
+ unsigned char *orig_zl = zl;
+ p = ziplistIndex(zl, 0);
+ zl = ziplistReplace(zl, p, (unsigned char*)"zoink", 5);
+ p = ziplistIndex(zl, 3);
+ zl = ziplistReplace(zl, p, (unsigned char*)"yy", 2);
+ p = ziplistIndex(zl, 1);
+ zl = ziplistReplace(zl, p, (unsigned char*)"65536", 5);
+ p = ziplistIndex(zl, 0);
+ assert(!memcmp((char*)p,
+ "\x00\x05zoink"
+ "\x07\xf0\x00\x00\x01" /* 65536 as int24 */
+ "\x05\x04quux" "\x06\x02yy" "\xff",
+ 23));
+ assert(zl == orig_zl); /* no reallocations have happened */
+ zfree(zl);
+ printf("SUCCESS\n\n");
+ }
+
+ printf("Replace with different size:\n");
+ {
+ zl = createList(); /* "hello", "foo", "quux", "1024" */
+ p = ziplistIndex(zl, 1);
+ zl = ziplistReplace(zl, p, (unsigned char*)"squirrel", 8);
+ p = ziplistIndex(zl, 0);
+ assert(!strncmp((char*)p,
+ "\x00\x05hello" "\x07\x08squirrel" "\x0a\x04quux"
+ "\x06\xc0\x00\x04" "\xff",
+ 28));
+ zfree(zl);
+ printf("SUCCESS\n\n");
+ }
+
printf("Regression test for >255 byte strings:\n");
{
char v1[257] = {0}, v2[257] = {0};
diff --git a/src/ziplist.h b/src/ziplist.h
index fff334cbd..9dc1061b0 100644
--- a/src/ziplist.h
+++ b/src/ziplist.h
@@ -53,6 +53,7 @@ unsigned int ziplistGet(unsigned char *p, unsigned char **sval, unsigned int *sl
unsigned char *ziplistInsert(unsigned char *zl, unsigned char *p, unsigned char *s, unsigned int slen);
unsigned char *ziplistDelete(unsigned char *zl, unsigned char **p);
unsigned char *ziplistDeleteRange(unsigned char *zl, int index, unsigned int num);
+unsigned char *ziplistReplace(unsigned char *zl, unsigned char *p, unsigned char *s, unsigned int slen);
unsigned int ziplistCompare(unsigned char *p, unsigned char *s, unsigned int slen);
unsigned char *ziplistFind(unsigned char *zl, unsigned char *p, unsigned char *vstr, unsigned int vlen, unsigned int skip);
unsigned int ziplistLen(unsigned char *zl);