Darrel Schneider created GEODE-9497:
---------------------------------------
Summary: RedisHash and RedisSortedSet both use a hashMap that uses
more memory than it needs to
Key: GEODE-9497
URL: https://issues.apache.org/jira/browse/GEODE-9497
Project: Geode
Issue Type: Bug
Components: redis
Reporter: Darrel Schneider
Both RedisHash and RedisSortedSet have an instance of
SizeableObject2ObjectOpenCustomHashMapWithCursor which can end up keeping some
objects alive that are not really needed. The memory used by these object is
currently not kept track of by geode's getSizeInBytes implementation.
The objects referenced by these three fields are the problem:
# *entries* an instance of
it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap.MapEntrySet
# *keys* an instance of
it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap.KeySet
# *values* an instance of an anonymous subclass of
it.unimi.dsi.fastutil.objects.AbstractObjectCollection
These fields get lazily initialized when their corresponding method is called.
Once they are initialized they stay that way forever. The object they reference
is immutable. Its only field is the implicit reference to its outer class
instance. Currently it looks like we have redis ops that can end up calling all
three of these methods in which case these objects would account for over 100
bytes on a 64-bit jvm.
It is not clear to me why the implementors of Object2ObjectOpenCustomHashMap
chose to cache this immutable object instead of just creating it every time.
They have no state to initialize when constructed so one thing we could do is
override the methods that cache it to either null out the cached field after
calling the super method, or have the method ignore the super implementation
and just create and returns an instance without caching. The cache fields
themselves would be a waste of memory in that case but only 24 bytes (12 with
compressed oops). To avoid this waste of memory we could just create our own
subclass of AbstractObject2ObjectMap that does not have these three fields. It
looks like we might be able to get rid of some other fields (one of them is to
support null keys which we never use). If we go that direction we should
probably force byte array keys instead of have a generic parameter and we could
then get rid of the strategy field.
Another possibility would be for our code that uses the map to only call
entrySet(), that way one instance would be cached instead of 3. You can always
get keys and values from the entrySet. Even better would be to add the methods
we need to our subclass of Object2ObjectOpenCustomHashMap named
SizeableObject2ObjectOpenCustomHashMapWithCursor. We did this for scan to have
a stable cursor and we could do it easily for the places we need keys, values,
or entries. It would be easy to add the foreach style of method directly on our
subclass and use it in RedisHash. If we do this then we should probably
override the other methods to throw an exception to prevent someone from
accidentally using them in the future.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
