[
https://issues.apache.org/jira/browse/SLING-12279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17846767#comment-17846767
]
Paul Bjorkstrand commented on SLING-12279:
------------------------------------------
[~pakira], you are correct that this does not fix that particular problem. The
problem that this attempts to address, is when the cache fills with
[circularly-referenced cached
models|https://sling.apache.org/documentation/bundles/models.html#a-note-about-cache-true-and-using-the-self-injector]
(most commonly when model marked with `cache=true`, and keeps a field with
`@Self`), this change should allow the GC to collect the cache when the
Resource Resolver or Request are closed/destroyed. That will eliminate a large
"sawtooth" GC graph.
Without this change, models that, directly or indirectly, reference their
adaptable will not be collected until there is memory pressure, and the GC
starts collecting SoftReference. I do believe with my proposed change, swapping
out the cache impl (currently just a `Map<Object, Map<Class<?>,
SoftReference<Object>>>` will be easier. A follow-on change could add in the
LRU portion of the cache impl in one place, rather than in the several it would
need today..
> Move Sling Model cache holder for Resource and ResourceResolver adaptables
> into the resource resolver property map
> ------------------------------------------------------------------------------------------------------------------
>
> Key: SLING-12279
> URL: https://issues.apache.org/jira/browse/SLING-12279
> Project: Sling
> Issue Type: Improvement
> Components: Sling Models
> Affects Versions: Models Implementation 1.6.4
> Reporter: Paul Bjorkstrand
> Priority: Major
> Attachments: cache-size.log, cachesize.log
>
>
> There have been several instances of issues with model caching over the
> years, the most recent being SLING-12259 and [this
> thread|https://www.mail-archive.com/dev@sling.apache.org/msg131926.html] from
> Jörg Hoh. These recent issues have been around cached items sticking
> around for too long. In that thread, it was discussed using
> {{[ResourceResolver#getPropertyMap()|https://github.com/apache/sling-org-apache-sling-api/blob/master/src/main/java/org/apache/sling/api/resource/ResourceResolver.java#L888]}}
> whenever possible. This binds the cache to the lifecycle of the
> ResourceResolver, avoiding the global cache altogether.
> After looking at the [current
> implementation|https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L341],
> there is already a divergence that binds the cache for request-based models
> to the request, by putting the cache in the request attribute.
> This proposes to do a similar change for Resource/Resolver based models, and
> use the resolver's property map to bind the cache for those adaptables to the
> lifecycle of the ResourceResolver. Resources are already (largely) bound to
> the lifecycle of the ResourceResolver that supplied them, and binding the
> models that come from these Resources seems to be a reasonable approach.
> This will greatly reduce the lifetime of model objects, reducing the
> likelihood of the JVM's GC being put under pressure in cases when cached
> models [reference the original adaptable using
> {{@Self}}|https://sling.apache.org/documentation/bundles/models.html#a-note-about-cache-true-and-using-the-self-injector].
> As a bonus, if the cache object implements {{Closeable}}, the Sling
> Implementation will call {{close()}} on the cache when the resolver is
> closed,, giving us further control over the lifetime of objects in the cache.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
