[ 
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)

Reply via email to