DonalEvans commented on a change in pull request #7219:
URL: https://github.com/apache/geode/pull/7219#discussion_r779046312
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -512,22 +505,27 @@ public long zrevrank(byte[] member) {
public static long zunionstore(RegionProvider regionProvider, RedisKey key,
List<ZKeyWeight> keyWeights,
ZAggregator aggregator) {
- MemberMap unionMembers = null;
- ScoreSet unionScores = null;
+ MemberMap unionMembers = new MemberMap(keyWeights.size());
Review comment:
This initial size is a bit misleading, since the number of keys passed
to the ZUNIONSTORE command doesn't technically have any relation to the number
of members in those keys. There's unfortunately not really a sensible way to
estimate an initial size for the `MemberMap` in this method, because even if we
use the size of the first set we find, the final size could be anywhere between
that size (if none of the other sets contain members not present in the first
set) and the combined size of all the sets passed to the command (if all of the
sets contain unique members), so some resizing of the `MemberMap` is quite
likely even if we use our best guess.
Just for clarity, it might be best to use an initial size of 0 here rather
than choosing an arbitrary number.
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -291,12 +291,15 @@ public static long zinterstore(RegionProvider
regionProvider, RedisKey key,
List<ZKeyWeight> keyWeights,
ZAggregator aggregator) {
List<RedisSortedSet> sets = new ArrayList<>(keyWeights.size());
+ MemberMap interMembers = new MemberMap(keyWeights.size());
+ ScoreSet interScores = new ScoreSet();
Review comment:
Rather than moving the definition of these variables up to here, they
can stay where they were originally on lines 312 and 313 and instead just
directly use `new ScoreSet()` and `new MemberMap(0)` in the
`sortedSetOpStoreResult()` method call. The initial size of the `MemberMap` in
`zinterstore()` should remain as `smallestSet.getSortedSetSize()` as that's the
largest size it's possible for the result to be.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
