dschneider-pivotal commented on a change in pull request #6726:
URL: https://github.com/apache/geode/pull/6726#discussion_r680063808



##########
File path: 
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -395,6 +395,31 @@ long zrevrank(byte[] member) {
     return null;
   }
 
+  List<byte[]> zpopmax(Region<RedisKey, RedisData> region, RedisKey key, int 
count) {
+    Iterator<AbstractOrderedSetEntry> scoresIterator =
+        scoreSet.getIndexRange(scoreSet.size() - 1, count, true);
+    List<byte[]> result = new ArrayList<>();
+
+    if (!scoresIterator.hasNext()) {
+      return result;

Review comment:
       could the canonical empty list be returned here?

##########
File path: 
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -395,6 +395,31 @@ long zrevrank(byte[] member) {
     return null;
   }
 
+  List<byte[]> zpopmax(Region<RedisKey, RedisData> region, RedisKey key, int 
count) {
+    Iterator<AbstractOrderedSetEntry> scoresIterator =
+        scoreSet.getIndexRange(scoreSet.size() - 1, count, true);
+    List<byte[]> result = new ArrayList<>();

Review comment:
       if you return the canonical empty list then you could delay the 
ArrayList allocation until after the empty check. Can you use the arguments 
passed to getIndexRange to compute a better initial size of this ArrayList?




-- 
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]


Reply via email to