sabbey37 commented on a change in pull request #6296:
URL: https://github.com/apache/geode/pull/6296#discussion_r628551741
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -46,16 +46,37 @@
import org.apache.geode.redis.internal.delta.RemsDeltaInfo;
public class RedisSet extends AbstractRedisData {
-
private HashSet<ByteArrayWrapper> members;
- @SuppressWarnings("unchecked")
+ // the following constants were calculated using reflection and math. you
can find the tests for
+ // these values in RedisSetTest, which show the way these numbers were
calculated. the constants
+ // have the advantage of saving us a lot of computation that would happen
every time a new key was
+ // added. if our internal implementation changes, these values may be
incorrect. the tests will
+ // catch this change. an increase in overhead should be carefully considered.
+ // Note: the per member overhead is known to not be constant. it changes as
more members are
+ // added, and/or as the members get longer
+ protected static final int BASE_REDIS_SET_OVERHEAD = 112;
+ protected static final int PER_MEMBER_OVERHEAD = 77;
+ protected static final int INTERNAL_HASH_SET_STORAGE_OVERHEAD = 86;
+
+ private int sizeInBytes;
+
RedisSet(Collection<ByteArrayWrapper> members) {
+ sizeInBytes += BASE_REDIS_SET_OVERHEAD;
Review comment:
Should we move this to the empty `RedisSet` constructor like we did for
`RedisHash`?
--
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:
[email protected]
