paul-bjorkstrand edited a comment on issue #6: SLING-7586 - using weak 
references as cache values to avoid memory leak
URL: 
https://github.com/apache/sling-org-apache-sling-models-impl/pull/6#issuecomment-391569488
 
 
   Not sure if this is the best approach. If the leak is related to the 
`adaptableCache`'s keys (classes) being strongly referenced, then the 
`adaptableCache` items (the values of the outer map, or `adapterCache`) should 
be 
[WeakHashMaps](https://docs.oracle.com/javase/8/docs/api/java/util/WeakHashMap.html),
 rather than having the result of the adaptation be kept in 
[WeakReferences](https://docs.oracle.com/javase/8/docs/api/java/lang/ref/WeakReference.html).
   
   Actually, thinking about how Felix (and any OSGi container) loads classes, 
that is probably the way to do it. If a class is collected, because its bundle 
was reloaded/reinstalled, then the old class (before the reload) is not capable 
of being collected by the GC, because they are strongly referenced by being the 
keys in the inner map.
   
   ~~~This PR has two issues, as far as I can see:~~~
   
   ~~~1. It does not resolve the memory leak issue, since the keys of the inner 
maps (the adaptableCache) are still strongly referenced by the map itself.~~~
   ~~~2. it adds an odd behavior/performance regression: the adaptableCache 
maps will have keys for given classes, but will have no values, as the model 
instances may have been (and likely will have been) collected by the GC, since 
they are weakly referenced, due to being put inside WeakReferences.~~~
   
   The field should still be typed as
   ```
   Map<Object, Map<Class<?>, Object>> adapterCache;
   ```
   and initialized by 
   ```
   adapterCache = Collections.synchronizedMap(new WeakHashMap<>());
   ```
   
   The values of the `adapterCache` should by typed as
   ```
   Map<Class<?>, Object> adaptableCache;
   ```
   and initialized by
   ```
   adaptableCache = Collections.synchronizedMap(new 
WeakHashMap<>(INNER_CACHE_INITIAL_CAPACITY));
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to