From 32f45215c33e8b8d801c4c339e18504918082936 Mon Sep 17 00:00:00 2001 From: Binbin Date: Mon, 29 May 2023 14:55:17 +0800 Subject: [PATCH] Try lazyfree temp zset in ZUNION / ZINTER / ZDIFF and optimize ZINTERCARD to avoid create temp zset (#12229) We check lazyfree_lazy_server_del in sunionDiffGenericCommand to see if we need to lazyfree the temp set. Now do the same in zunionInterDiffGenericCommand to lazyfree the temp zset. This is a minor change, follow #5903. Also improved the comments. Additionally, avoid creating unused zset object in ZINTERCARD, results in some 10% performance improvement. --- src/t_set.c | 4 ++-- src/t_zset.c | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/t_set.c b/src/t_set.c index fb85d220f..7e3a9ce81 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -1540,8 +1540,8 @@ void sunionDiffGenericCommand(client *c, robj **setkeys, int setnum, } } - /* We need a temp set object to store our union. If the dstkey - * is not NULL (that is, we are inside an SUNIONSTORE operation) then + /* We need a temp set object to store our union/diff. If the dstkey + * is not NULL (that is, we are inside an SUNIONSTORE/SDIFFSTORE operation) then * this set object will be the resulting object to set into the target key*/ dstset = createIntsetObject(); diff --git a/src/t_zset.c b/src/t_zset.c index 6f0306c65..7717a4a14 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -2575,8 +2575,8 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in zsetopval zval; sds tmp; size_t maxelelen = 0, totelelen = 0; - robj *dstobj; - zset *dstzset; + robj *dstobj = NULL; + zset *dstzset = NULL; zskiplistNode *znode; int withscores = 0; unsigned long cardinality = 0; @@ -2686,8 +2686,14 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in qsort(src,setnum,sizeof(zsetopsrc),zuiCompareByCardinality); } - dstobj = createZsetObject(); - dstzset = dstobj->ptr; + /* We need a temp zset object to store our union/inter/diff. If the dstkey + * is not NULL (that is, we are inside an ZUNIONSTORE/ZINTERSTORE/ZDIFFSTORE operation) then + * this zset object will be the resulting object to zset into the target key. + * In SINTERCARD case, we don't need the temp obj, so we can avoid creating it. */ + if (!cardinality_only) { + dstobj = createZsetObject(); + dstzset = dstobj->ptr; + } memset(&zval, 0, sizeof(zval)); if (op == SET_OP_INTER) { @@ -2826,6 +2832,7 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in server.dirty++; } } + decrRefCount(dstobj); } else if (cardinality_only) { addReplyLongLong(c, cardinality); } else { @@ -2846,8 +2853,9 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in if (withscores) addReplyDouble(c,zn->score); zn = zn->level[0].forward; } + server.lazyfree_lazy_server_del ? freeObjAsync(NULL, dstobj, -1) : + decrRefCount(dstobj); } - decrRefCount(dstobj); zfree(src); }