Unverified Commit 26ef2846 authored by Moti Cohen's avatar Moti Cohen Committed by GitHub
Browse files

Optimize ZUNION[STORE] by avoiding redundant temporary dict usage (#13566)

This PR is based on valkey-io/valkey#829

Previously, ZUNION and ZUNIONSTORE commands used a temporary accumulator dict
and at the end copied it as-is to dstzset->dict. This PR removes accumulator and directly
stores into dstzset->dict, eliminating the extra copy.

Co-authored-by: Rayacoo zisong.cw@alibaba-inc.com
parent 5f28bd96
/* /* t_zset.c -- zset data type implementation.
* Copyright (c) 2009-current, Redis Ltd. *
* Copyright (c) 2009-2012, Pieter Noordhuis <pcnoordhuis at gmail dot com> * Copyright (c) 2009-Present, Redis Ltd.
* All rights reserved. * All rights reserved.
* *
* Redistribution and use in source and binary forms, with or without * Copyright (c) 2024-present, Valkey contributors.
* modification, are permitted provided that the following conditions are met: * All rights reserved.
* *
* * Redistributions of source code must retain the above copyright notice, * Licensed under your choice of the Redis Source Available License 2.0
* this list of conditions and the following disclaimer. * (RSALv2) or the Server Side Public License v1 (SSPLv1).
* * Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* * Neither the name of Redis nor the names of its contributors may be used
* to endorse or promote products derived from this software without
* specific prior written permission.
* *
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" * Portions of this file are available under BSD3 terms; see REDISCONTRIBUTIONS for more information.
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/ */
/*----------------------------------------------------------------------------- /*-----------------------------------------------------------------------------
...@@ -2611,16 +2595,6 @@ static void zdiff(zsetopsrc *src, long setnum, zset *dstzset, size_t *maxelelen, ...@@ -2611,16 +2595,6 @@ static void zdiff(zsetopsrc *src, long setnum, zset *dstzset, size_t *maxelelen,
} }
} }
dictType setAccumulatorDictType = {
dictSdsHash, /* hash function */
NULL, /* key dup */
NULL, /* val dup */
dictSdsKeyCompare, /* key compare */
NULL, /* key destructor */
NULL, /* val destructor */
NULL /* allow to expand */
};
/* The zunionInterDiffGenericCommand() function is called in order to implement the /* The zunionInterDiffGenericCommand() function is called in order to implement the
* following commands: ZUNION, ZINTER, ZDIFF, ZUNIONSTORE, ZINTERSTORE, ZDIFFSTORE, * following commands: ZUNION, ZINTER, ZDIFF, ZUNIONSTORE, ZINTERSTORE, ZDIFFSTORE,
* ZINTERCARD. * ZINTERCARD.
...@@ -2816,7 +2790,6 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in ...@@ -2816,7 +2790,6 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in
zuiClearIterator(&src[0]); zuiClearIterator(&src[0]);
} }
} else if (op == SET_OP_UNION) { } else if (op == SET_OP_UNION) {
dict *accumulator = dictCreate(&setAccumulatorDictType);
dictIterator *di; dictIterator *di;
dictEntry *de, *existing; dictEntry *de, *existing;
double score; double score;
...@@ -2824,7 +2797,7 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in ...@@ -2824,7 +2797,7 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in
if (setnum) { if (setnum) {
/* Our union is at least as large as the largest set. /* Our union is at least as large as the largest set.
* Resize the dictionary ASAP to avoid useless rehashing. */ * Resize the dictionary ASAP to avoid useless rehashing. */
dictExpand(accumulator,zuiLength(&src[setnum-1])); dictExpand(dstzset->dict,zuiLength(&src[setnum-1]));
} }
/* Step 1: Create a dictionary of elements -> aggregated-scores /* Step 1: Create a dictionary of elements -> aggregated-scores
...@@ -2839,7 +2812,7 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in ...@@ -2839,7 +2812,7 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in
if (isnan(score)) score = 0; if (isnan(score)) score = 0;
/* Search for this element in the accumulating dictionary. */ /* Search for this element in the accumulating dictionary. */
de = dictAddRaw(accumulator,zuiSdsFromValue(&zval),&existing); de = dictAddRaw(dstzset->dict,zuiSdsFromValue(&zval),&existing);
/* If we don't have it, we need to create a new entry. */ /* If we don't have it, we need to create a new entry. */
if (!existing) { if (!existing) {
tmp = zuiNewSdsFromValue(&zval); tmp = zuiNewSdsFromValue(&zval);
...@@ -2849,7 +2822,7 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in ...@@ -2849,7 +2822,7 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in
totelelen += sdslen(tmp); totelelen += sdslen(tmp);
if (sdslen(tmp) > maxelelen) maxelelen = sdslen(tmp); if (sdslen(tmp) > maxelelen) maxelelen = sdslen(tmp);
/* Update the element with its initial score. */ /* Update the element with its initial score. */
dictSetKey(accumulator, de, tmp); dictSetKey(dstzset->dict, de, tmp);
dictSetDoubleVal(de,score); dictSetDoubleVal(de,score);
} else { } else {
/* Update the score with the score of the new instance /* Update the score with the score of the new instance
...@@ -2866,21 +2839,15 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in ...@@ -2866,21 +2839,15 @@ void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIndex, in
} }
/* Step 2: convert the dictionary into the final sorted set. */ /* Step 2: convert the dictionary into the final sorted set. */
di = dictGetIterator(accumulator); di = dictGetIterator(dstzset->dict);
/* We now are aware of the final size of the resulting sorted set,
* let's resize the dictionary embedded inside the sorted set to the
* right size, in order to save rehashing time. */
dictExpand(dstzset->dict,dictSize(accumulator));
while((de = dictNext(di)) != NULL) { while((de = dictNext(di)) != NULL) {
sds ele = dictGetKey(de); sds ele = dictGetKey(de);
score = dictGetDoubleVal(de); score = dictGetDoubleVal(de);
znode = zslInsert(dstzset->zsl,score,ele); znode = zslInsert(dstzset->zsl,score,ele);
dictAdd(dstzset->dict,ele,&znode->score); dictSetVal(dstzset->dict,de,&znode->score);
} }
dictReleaseIterator(di); dictReleaseIterator(di);
dictRelease(accumulator);
} else if (op == SET_OP_DIFF) { } else if (op == SET_OP_DIFF) {
zdiff(src, setnum, dstzset, &maxelelen, &totelelen); zdiff(src, setnum, dstzset, &maxelelen, &totelelen);
} else { } else {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment