[
https://issues.apache.org/jira/browse/SLING-12217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072894#comment-18072894
]
Bhavik Kothari commented on SLING-12217:
----------------------------------------
Thank you for pointing me to
[JavaUseProvider::loadObject()|https://github.com/apache/sling-org-apache-sling-scripting-sightly/blob/f7f73e2feefb39a4496a85a093fe834aca92fb01/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/JavaUseProvider.java#L178-L206]
! I was so laser-focused on the adapter's internal loops that I completely
missed the starting invocation
Looking at the code again, your {{tryCreateModel}} idea is a brilliant,
stateless way to optimize the 'hits' in that waterfall pattern, I also
completely agree with hesitation around caches—global caches usually just bring
nightmares with stale data and memory leaks
However, looking right at the start of that method, {{isModelClass(cls)}}
executes before the waterfall even begins, If a component _doesn't_ have a
model, {{isModelClass}} still triggers the heavy JCR traversals just to confirm
the 'miss'
Because cache is strictly bound to {{{}resolver.getPropertyMap(){}}}, it dies
instantly at the end of the HTTP request, which safely avoids those usual stale
data/memory leak risks while blocking the duplicate 'miss' traversal
What do you think about a two-way approach? We keep this request-scoped cache
to shield against the 'misses' and I will actually raise a PR for your
{{tryCreateModel}} API change shortly so we can look at the code!
Once that is up, we can look at them side-by-side and decide the best path
forward: whether we want to keep both (using the API for hits and this
request-cache for misses), or if we should put this cache PR on hold to avoid
the risk entirely
> Reduce resource lookups in Sling Model resolution
> -------------------------------------------------
>
> Key: SLING-12217
> URL: https://issues.apache.org/jira/browse/SLING-12217
> Project: Sling
> Issue Type: Task
> Components: Sling Models
> Affects Versions: Models Implementation 1.6.4
> Reporter: Joerg Hoh
> Priority: Major
>
> While analyzing repository access, I found a pattern in the Scripting HTL
> implementation, which leads to multiple invocations of the same check on the
> same resources in the Sling Model ModelAdapterFactory.
> Invocation at
> [JavaUseProvider.loadObject()|https://github.com/apache/sling-org-apache-sling-scripting-sightly/blob/690a818692bd08c3f6a49e842ea530cc3e60e3ad/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/use/JavaUseProvider.java#L172]:
> * modelAdapterFactory.canCreateModelFromAdaptable()
> * modelAdapterFactory.createModel
> those methods directly or indirectly call
> [org.apache.sling.models.impl.AdapterImplementations.lookup()|https://github.com/apache/sling-org-apache-sling-models-impl/blob/cf088713c402177b9d96a5229567804510ef9918/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java#L192],
> which calls
> [getModelClassForResource|https://github.com/apache/sling-org-apache-sling-models-impl/blob/cf088713c402177b9d96a5229567804510ef9918/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java#L302];
> and there multiple resources are resolved, based on the provided
> resourceType.
> I benchmarked this code using a simple AEM sample page (WKND) using HTL and
> Sling Models. I found this code is responsible to the creation of 712 JCR
> Node resource objects (the total page rendering triggered the creation of
> more than 10k, so these are around 7%). When deduplicating the requested
> paths, I found that only 36 distinct paths were resolved with these 712 JCR
> Node resources.
> That means that with proper caching (probably using the
> ResourceResolver.getPropertyMap) around 6.5% of repository access could be
> avoided, leading to a page rendering improvement in about the same range.
>
>
>
>
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
