sling-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] paul-bjorkstrand commented on issue #6: SLING-7586 - using weak references as cache values to avoid memory leak
Date Thu, 24 May 2018 02:41:27 GMT
paul-bjorkstrand commented 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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message