DonalEvans commented on a change in pull request #6701:
URL: https://github.com/apache/geode/pull/6701#discussion_r672429805



##########
File path: 
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java
##########
@@ -237,35 +237,32 @@ public int hstrlen(byte[] field) {
     return new ArrayList<>(hash.keySet());
   }
 
-  public ImmutablePair<Integer, List<byte[]>> hscan(Pattern matchPattern,
-      int count,
-      int cursor) {
-
-    ArrayList<byte[]> resultList = new ArrayList<>(count + 2);
+  public ImmutablePair<Integer, List<ImmutablePair<byte[], byte[]>>> 
hscan(Pattern matchPattern,
+      int count, int cursor) {
+    // No need to allocate more space than it's possible to use given the size 
of the hash
+    int initialCapacity = Math.min(count, hash.size());
+    List<ImmutablePair<byte[], byte[]>> resultList = new 
ArrayList<>(initialCapacity);
     do {
       cursor = hash.scan(cursor, 1,
           (list, key, value) -> addIfMatching(matchPattern, list, key, value), 
resultList);
-    } while (cursor != 0 && resultList.size() < (count * 2));
+    } while (cursor != 0 && resultList.size() < count);
 
     return new ImmutablePair<>(cursor, resultList);
   }
 
-  private void addIfMatching(Pattern matchPattern, List<byte[]> resultList, 
byte[] key,
-      byte[] value) {
+  private void addIfMatching(Pattern matchPattern, List<ImmutablePair<byte[], 
byte[]>> resultList,
+      byte[] key, byte[] value) {
     if (matchPattern != null) {
       if (matchPattern.matcher(bytesToString(key)).matches()) {
-        resultList.add(key);
-        resultList.add(value);
+        resultList.add(new ImmutablePair<>(key, value));

Review comment:
       Yeah, I think I tend to agree with you on this one. I suspect that the 
vast majority of users would rather their cache be faster than that it be able 
to handle an improbably large number of keys/values. I'll rework this part of 
the code and see about maybe introducing some error logging if we hit that 
unlikely scenario.




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