Hi Dominik, Yes, the ResourceProvider API would have to be changed (or a new API, e.g. OptimizableResourceProvider) be introduced).
If that isn’t an option, then we could either implement a “minicache” with a ThreadLocal (which comes at a cost as well), think about an even uglier workaround with the Map<String, String> parameters or consider my initial proposal again. Carsten’s objection was the following: “You don't know whether the parent or any child resource is backed by JCR. It could be that the parent is delivered by a different resource provider or that child nodes are a combination of resource providers." Would it be an option to create a utility which can be used in getParent/getChild/getChildren to check if the parent/child path is handled with the same resource provider? If this check returns true, then it would execute the optimised methods and otherwise fall back to the general methods. I would prefer a new API though. - Joel On 09/04/15 10:49, "Dominik Süß" <[email protected]> wrote: >Hi Joel, > >your idea is close to what I was thinking of before but the issue is that >the ResourceProvider interface is does not provide this mechanism and the >ResourceResolverImpl therefore does not pass the BaseResource to the >ResourceProviders but as you stated calculate the path. > >This is why I thought of having some kind of minicache (more a prefetching >mechanism) for resourceproviders bound to the resourceResolver (and >therefore living and dying with the resourceResolver). So this is a cache >that should auto clean up and should not do any harm. > >+1 anyways for some benchmark project to have a reliable, trackable >measurement in sling for any solution applied. > >Cheers >Dominik > >On Thu, Apr 9, 2015 at 10:12 AM, Joel Richard <[email protected]> wrote: > >> Hi, >> >> Yesterday evening I had another idea which might be easier to implement. >> But first a few observations: >> >> jcrNode.getParent() is about 10 times faster than resource.getParent(). >> The same is true for getChild() vs getNode(). The difference will become >> less as soon as we have fixed some other issues, but since even >> jcrSession.getNode(parentPath) is about 3.5 times slower than >> jcrNode.getParent(), it is unlikely that resource.getParent() will ever >>be >> as fast as jcrNode.getParent() without changing the implementation. >> >> getChild is implemented with getResourceResolver().getResource(this, >> relPath) and getParent could be implemented with getResource(this, >>".."). >> The interesting thing here is that getResource allows to pass the base >> resource to the ResourceResolver. >> >> At the moment this is only used to compute the absolute path. If the >>base >> resource was passed to ResourceProviderEntry#getInternalResource and >>from >> there to the JcrResourceProvider, then the JcrResourceProvider could >>first >> check whether the resource is an instance of JcrItemResource and whether >> the paths are related. In this case it could simply get the >>property/node >> and use node.getParent/node.getNode as an optimisation. >> >> With this approach no additional caches would have to be implemented and >> different resource providers would work together as today. >> >> - Joel >> >> >> On 09/04/15 06:55, "Carsten Ziegeler" <[email protected]> wrote: >> >> >Am 09.04.15 um 08:06 schrieb Dominik Süß: >> >> Hi Carsten, hi Joel, >> >> >> >> AFAIU the cost intensive part is the call of createResource() at [0] >> >>that >> >> will do a look up by path but not utilize the node of the resource >> >>issuing >> >> this lookup through getParent (indirected via resourceResolver >>lookup by >> >> the AbstractResource). >> >> >> >> What would you think about "caching" the parentNode when getParent() >>is >> >> called for the ResourceProvider so that as the resourceResolver >>consults >> >> the JCRResourceProvider in the follow up call the parentNode is >>already >> >> present. >> >> >> >> As we most probably don't have an extremely wild mix of different >> >> resourceProviders this caching should only render unnecessary in the >> >>rare >> >> cases where we have a different ResourceProvider at a higher level >> >>without >> >> doing any harm but a little bit of memory consumption (feature could >>be >> >> implemented with a on-off switch ;) ) >> >> >> > >> >Well, in general I'm against adding small caches all over the place as >> >this >> >will most likely turn into pain over time. And usually it works in one >> >use case but fails (or turns into worse performance) in other use >>cases. >> >This might not be the case here. >> > >> >I think, before we do any changes we should have test cases where we >>can >> >check the results against. I trust Joel's analysis, however we don't >> >have any benchmarks in the Sling project and it's very hard atm for the >> >Sling community to see the problem and to verify the implications of >> >a change - being it good or bad. >> > >> >Therefore, let's create a benchmark project first and use that for >> >improvements. >> > >> >Regards >> >Carsten >> >-- >> >Carsten Ziegeler >> >Adobe Research Switzerland >> >[email protected] >>
