[jira] [Comment Edited] (SLING-10899) Result of JcrNodeResource#adaptTo(ValueMap.class) should be cached

["Henry Kuijpers (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-10899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17437268#comment-17437268 ] 

Henry Kuijpers edited comment on SLING-10899 at 11/2/21, 10:33 AM:
-------------------------------------------------------------------

[~cziegeler], I see. However, the performance impact is quite drastic. Not only in our very specific situation, but also in the Sling product itself: With every filter, with every servlet, with every OSGi service, ... accessing the valuemap of the Resource, a new one is created and therefore, fetching properties from it will always reach out to the repository for the first time, since the cache will always be empty in that case. 

ValueMap objects are not shared in general, not because technically it's impossible to do so (if you have the reference, you can just use it), but rather because the various APIs etc get a Resource-object to work with (and therefore you will always have to call getValueMap (or adaptTo) to obtain it).

I think the impact on performance with this improvement will be quite noticeable.

Maybe the ResourceResolver code can be updated, so that it can track resources that have been adapted to a ModifiableValueMap? So that, on commit, it can invalidate the internal caches?


was (Author: henry kuijpers):
[~cziegeler], I see. However, the performance impact is quite drastic. Not only in our very specific situation, but also in the Sling product itself: With every filter, with every servlet, with every OSGi service, ... accessing the valuemap of the Resource, a new one is created and therefore, fetching properties from it will always reach out to the repository. 

ValueMap objects are not shared in general, not because technically it's impossible to do so (if you have the reference, you can just use it), but rather because the various APIs etc get a Resource-object to work with (and therefore you will always have to call getValueMap (or adaptTo) to obtain it).

I think the impact on performance with this improvement will be quite noticeable.

Maybe the ResourceResolver code can be updated, so that it can track resources that have been adapted to a ModifiableValueMap? So that, on commit, it can invalidate the internal caches?

> 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
>            Assignee: Carsten Ziegeler
>            Priority: Major
>             Fix For: JCR Resource 3.0.24
>
>
> 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.3.4#803005)