[
https://issues.apache.org/jira/browse/SLING-10899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17437202#comment-17437202
]
Carsten Ziegeler commented on SLING-10899:
------------------------------------------
We don't do anything special for commit/refresh today. The problem exists
regardless whether we cache the value map or not. Granted, with caching the
value map, the problem might occur more often. But it can already occur today.
So we have a couple of problems: there might be stale value maps - but we also
support deep tree access/modifications through (modifiable) value maps - which
allows for cross resource changes.
I'm wondering now if it is really worth caching the value map as it seems to
create more problems than it actually solves?
In theory we could return the same object for value map and modifiable value
map and just make it immutable for value map. This way changes to the
modifiable map would be visibile to the read-only value map. But I'Ve also seen
code which actually alters a value map and expects that the changes do not end
up persisted. So we would break that (rather strange) use case.
> 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)
