jdeppe-pivotal commented on a change in pull request #7319:
URL: https://github.com/apache/geode/pull/7319#discussion_r795785591



##########
File path: 
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -245,33 +244,25 @@ static int setOpStoreResult(RegionProvider 
regionProvider, RedisKey destinationK
     return scanResult;
   }
 
-  public Collection<byte[]> spop(Region<RedisKey, RedisData> region, RedisKey 
key, int popCount) {
-    int originalSize = scard();
-    if (originalSize == 0) {
-      return emptyList();
-    }
-
-    if (popCount >= originalSize) {
-      region.remove(key, this);
-      return members;
-    }
-
-    List<byte[]> popped = new ArrayList<>();
-    byte[][] setMembers = members.toArray(new byte[originalSize][]);
-    Random rand = new Random();
-    while (popped.size() < popCount) {
-      int idx = rand.nextInt(originalSize);
-      byte[] memberToPop = setMembers[idx];
-      if (memberToPop != null) {
-        setMembers[idx] = null;
-        popped.add(memberToPop);
-        membersRemove(memberToPop);
+  public List<byte[]> spop(int count, Region<RedisKey, RedisData> region, 
RedisKey key) {
+    List<byte[]> result = new ArrayList<>();
+    if (members.size() <= count) {
+      result.addAll(members);
+      members.clear();
+    } else {
+      Random rand = new Random();
+      while (result.size() != count) {
+        int randIndex = rand.nextInt(members.getBackingArrayLength());
+        byte[] member = members.getFromBackingArray(randIndex);
+        if (member != null) {

Review comment:
       I see how this works now. I think Darrel's suggestion is good especially 
when the backing array is fairly sparse. I'm not sure if I read the code 
properly, but my back-of-the-napkin calculation was that for an initial size of 
100 elements the backing array would have a size of 256. With a load factor of 
0.75, the worst case fill percentage is about 37.5%.




-- 
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: notifications-unsubscr...@geode.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to