[
https://issues.apache.org/jira/browse/SLING-10899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17770335#comment-17770335
]
Joerg Hoh commented on SLING-10899:
-----------------------------------
[~Henry Kuijpers] I see the problem with the caching of the results of this
adaption, as the cache must be limited in size (e.g 100 elements or 500k) ,
otherwise it can easily blow the JVM. This naturally limits the effectiveness,
because depending on the access patterns and the cache eviction strategy you
benefit from it only when you access a few resources constantly. Or if your
code access only a small amount of resources at a time.
I would say that in both cases you as the consumer of the API are in a much
better position to decide if you want to do that adaption repeatedly and if it
makes sense to cache the resulting ValueMaps in an application-layer cache and
thus avoid them.
And yes, this definitely makes it much harder to optimize performance, because
you need to look at each location independently, and there is no immediate
performance boost for each functionality doing such an adaption. But in my
opinion the effectiveness of such a cache is not guaranteed, and the memory
usage can be significant.
> Result of JcrNodeResource#adaptTo(ValueMap.class) should be cached
> ------------------------------------------------------------------
>
> Key: SLING-10899
> URL: https://issues.apache.org/jira/browse/SLING-10899
> Project: Sling
> Issue Type: Improvement
> Components: JCR
> Affects Versions: JCR Resource 3.0.22
> Reporter: Henry Kuijpers
> Priority: Major
>
> I was performance testing our application. There is a specific part of the
> code where we have a set of ~500 resources and we sort them based on a few
> properties of the resources. Multiple properties are used (which results in
> multiple calls to getValueMap). The sorting is done using a comparator, so
> the logic that determines the order is accessed numerous times.
> We've seen quite a decrease in performance when doing this with Resources
> that are of type JcrNodeResource. Turns out that the result of getValueMap
> (or adaptTo((Value)Map.class)) is not cached.
> As you can see in the adaptTo()-method implementation:
> https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/master/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResource.java#L136
> this call bypasses the AdapterFactory framework and also bypasses any
> caching that happens over there.
> What's even more unfortunate, is that JcrValueMap is implementing caching via
> https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/6e13f4d315ddf2804d2e16c55faee18e805b618e/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java#L49
> . That caching is not leveraged properly, because every call to getValueMap
> returns a new JcrValueMap, instead of reusing a previously created one.
> One change that can be done is maybe converting the logic inside the
> adaptTo()-method to a dedicated AdapterFactory implementation, so that
> caching is done by default?
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
