upthewaterspout commented on a change in pull request #6487:
URL: https://github.com/apache/geode/pull/6487#discussion_r634714167
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -340,7 +306,7 @@ long srem(ArrayList<ByteArrayWrapper> membersToRemove,
Region<RedisKey, RedisDat
* @return a set containing all the members in this set
*/
@VisibleForTesting
- Set<ByteArrayWrapper> smembers() {
+ public Set<byte[]> smembers() {
return new HashSet<>(members);
Review comment:
I would actually suggest changing this to
`Collections.unmodifiableSet(members)` here. This is an expensive operation, so
it would be better if we only make a copy of this set where we actually need to
modify it. Or only currently supported command, SMEMBERS, doesn't need this
extra copying.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
