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]
