[jira] [Closed] (GEODE-9497) RedisHash and RedisSortedSet both use a hashMap that uses more memory than it needs to

["Owen Nichols (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GEODE-9497?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Owen Nichols closed GEODE-9497.
-------------------------------

> 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: Improvement
>          Components: redis
>            Reporter: Darrel Schneider
>            Assignee: Darrel Schneider
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.15.0
>
>
> 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.20.7#820007)