[
https://issues.apache.org/jira/browse/GEODE-9345?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Donal Evans updated GEODE-9345:
-------------------------------
Description:
The implementation of the (unsupported at time of ticket creation) sunion,
sunionstore, sdiff, sdiffstore, sinter and sinterstore commands includes
potentially expensive set copying as part of the calls to {{smembers()}} and
{{internalsmembers()}}:
{code:java}
@Override
public Set<byte[]> smembers(RedisKey key) {
return stripedExecute(key, () -> new HashSet<>(getRedisSet(key,
true).smembers()));
}
@Override
public Set<byte[]> internalsmembers(RedisKey key) {
return stripedExecute(key, () -> new HashSet<>(getRedisSet(key,
false).smembers()));
}{code}
and in {{computeSetOp()}}:
{code:java}
if (result == null) {
result = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
} else {
switch (setOp) {
case UNION:
result.addAll(set);
break;
case INTERSECTION:
set = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
result.retainAll(set);
break;
case DIFF:
set = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
result.removeAll(set);
break;
}
}{code}
due to the need to use a custom hash set implementation to properly handle
equality comparisons between byte arrays, but the current implementation used,
ObjectOpenCustomHashSet, causes serialization errors when returned from
{{smembers()}} and {{internalsmembers()}}.
To avoid unnecessary set copying, a way should be found to allow an
{{ObjectOpenCustomHashSet}} to be returned from {{smembers()}} and
{{internalsmembers()}} without causing serialization errors. The best approach
would probably be to have the implementation of {{ObjectOpenCustomHashSet}}
also implement {{DataSerializableFixedID}}, which would allow it to be
efficiently serialized in a backwards-compatible way.
was:
The implementation of the (unsupported at time of ticket creation) sunion,
sunionstore, sdiff, sdiffstore, sinter and sinterstore commands includes
potentially expensive set copying as part of the calls to {{smembers()}} and
{{internalsmembers()}}:
{code:java}
@Override
public Set<byte[]> smembers(RedisKey key) {
return stripedExecute(key, () -> new HashSet<>(getRedisSet(key,
true).smembers()));
}
@Override
public Set<byte[]> internalsmembers(RedisKey key) {
return stripedExecute(key, () -> new HashSet<>(getRedisSet(key,
false).smembers()));
}{code}
and in {{computeSetOp()}}:
{code:java}
if (result == null) {
result = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
} else {
switch (setOp) {
case UNION:
result.addAll(set);
break;
case INTERSECTION:
set = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
result.retainAll(set);
break;
case DIFF:
set = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
result.removeAll(set);
break;
}
}{code}
due to the need to use a custom hash set implementation to properly handle
equality comparisons between byte arrays, but the current implementation used,
ObjectOpenCustomHashSet, causes serialization errors when returned from
{{smembers()}} and {{internalsmembers()}}.
To avoid unnecessary set copying, a way should be found to allow an
{{ObjectOpenCustomHashSet}} to be returned from {{smembers()}} and
{{internalsmembers()}} without causing serialization errors.
> Refactor RedisSet set operations to not do so much set copying
> --------------------------------------------------------------
>
> Key: GEODE-9345
> URL: https://issues.apache.org/jira/browse/GEODE-9345
> Project: Geode
> Issue Type: Improvement
> Components: redis
> Affects Versions: 1.15.0
> Reporter: Donal Evans
> Priority: Major
> Labels: redis
>
> The implementation of the (unsupported at time of ticket creation) sunion,
> sunionstore, sdiff, sdiffstore, sinter and sinterstore commands includes
> potentially expensive set copying as part of the calls to {{smembers()}} and
> {{internalsmembers()}}:
>
> {code:java}
> @Override
> public Set<byte[]> smembers(RedisKey key) {
> return stripedExecute(key, () -> new HashSet<>(getRedisSet(key,
> true).smembers()));
> }
> @Override
> public Set<byte[]> internalsmembers(RedisKey key) {
> return stripedExecute(key, () -> new HashSet<>(getRedisSet(key,
> false).smembers()));
> }{code}
> and in {{computeSetOp()}}:
>
> {code:java}
> if (result == null) {
> result = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
> } else {
> switch (setOp) {
> case UNION:
> result.addAll(set);
> break;
> case INTERSECTION:
> set = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
> result.retainAll(set);
> break;
> case DIFF:
> set = new ObjectOpenCustomHashSet<>(set, ByteArrays.HASH_STRATEGY);
> result.removeAll(set);
> break;
> }
> }{code}
>
> due to the need to use a custom hash set implementation to properly handle
> equality comparisons between byte arrays, but the current implementation
> used, ObjectOpenCustomHashSet, causes serialization errors when returned from
> {{smembers()}} and {{internalsmembers()}}.
> To avoid unnecessary set copying, a way should be found to allow an
> {{ObjectOpenCustomHashSet}} to be returned from {{smembers()}} and
> {{internalsmembers()}} without causing serialization errors. The best
> approach would probably be to have the implementation of
> {{ObjectOpenCustomHashSet}} also implement {{DataSerializableFixedID}}, which
> would allow it to be efficiently serialized in a backwards-compatible way.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
