Re: Caching for ResourceResolver.map()

2009-06-17 Thread Jukka Zitting
Hi,

On Thu, Jun 11, 2009 at 3:08 PM, Felix Meschberger wrote:
> Not sure, whether it really is the best approach. Though it is true,
> that the ResourceResolver instance is created on each request, it is
> still created from the JcrResourceResolverFactory, which might as well
> be the holder of the cache and provide the user-specific subset of the
> cache to the per-request-ResourceResolver.

I explored this idea a bit, and came up with a patch I submitted in SLING-1010.

BR,

Jukka Zitting


Re: Caching for ResourceResolver.map()

2009-06-11 Thread Dominik Süß
Hi,

On Thu, Jun 11, 2009 at 3:08 PM, Felix Meschberger wrote:

>
>
> Not sure, whether it really is the best approach. Though it is true,
> that the ResourceResolver instance is created on each request, it is
> still created from the JcrResourceResolverFactory, which might as well
> be the holder of the cache and provide the user-specific subset of the
> cache to the per-request-ResourceResolver.


It sounds a bit strange to have a factory holding a cache.
IMHO caching shouldn't be too hardwired since there are always situations
where the caching as it's implemented doesn't fit the requirements.
I think it would be nice to have a global ResourceResolver cache like you
described which each ResourceResolver can adapt but does not have to
(configurable).

WDYT?

Best regards,
Dominik


Re: Caching for ResourceResolver.map()

2009-06-11 Thread Felix Meschberger


Jukka Zitting schrieb:
> Hi,
> 
> On Wed, Jun 10, 2009 at 3:10 PM, Carsten Ziegeler wrote:
>> While caching in the resource resolver might be a good idea anyway,
>> perhaps caching one layer above might give even more benefit. Instead
>> of querying the resource resolver a lot, caching above the resolver
>> would even avoid querying the resolver.
> 
> This turns out to be the best approach in my case. Especially since a
> new ResourceResolver gets instantiated for each new request, which
> makes it more difficult to add global caching.

Not sure, whether it really is the best approach. Though it is true,
that the ResourceResolver instance is created on each request, it is
still created from the JcrResourceResolverFactory, which might as well
be the holder of the cache and provide the user-specific subset of the
cache to the per-request-ResourceResolver.

> 
> However, since the resolution process depends on access rights, I
> currently need to do something like this to include the potential
> username in the cache key:
> 
> String user = "";
> Session session = resourceResolver.adaptTo(Session.class);
> if (session != null) {
> user = session.getUserId();
> }
> 
> This seems a bit fragile, so I was wondering if getting the username
> from the request would work better:
> 
> String user = "";
> Principal principal = request.getUserPrincipal();
> if (principal != null) {
> user = principal.getName();
> }
> 
> I guess there are cases where the JCR Session (or whatever is used for
> resource resolution) is authenticated using something else than the
> user principal associated with the HTTP request.

You might want to use request.getRemoteUser(), which is backed by
Session.getUserId().

> 
> Ultimately we should probably solve the performance issue on the
> repository layer by making path lookups (especially negative ones)
> blazingly fast. They're already pretty good in Jackrabbit, but for my
> use case I'd need about an order of magnitude more performance. With a
> simple high-level cache I was able to achieve this.

oh yeah ;-)

Regards
Felix


Re: Caching for ResourceResolver.map()

2009-06-11 Thread Jukka Zitting
Hi,

On Wed, Jun 10, 2009 at 3:10 PM, Carsten Ziegeler wrote:
> While caching in the resource resolver might be a good idea anyway,
> perhaps caching one layer above might give even more benefit. Instead
> of querying the resource resolver a lot, caching above the resolver
> would even avoid querying the resolver.

This turns out to be the best approach in my case. Especially since a
new ResourceResolver gets instantiated for each new request, which
makes it more difficult to add global caching.

However, since the resolution process depends on access rights, I
currently need to do something like this to include the potential
username in the cache key:

String user = "";
Session session = resourceResolver.adaptTo(Session.class);
if (session != null) {
user = session.getUserId();
}

This seems a bit fragile, so I was wondering if getting the username
from the request would work better:

String user = "";
Principal principal = request.getUserPrincipal();
if (principal != null) {
user = principal.getName();
}

I guess there are cases where the JCR Session (or whatever is used for
resource resolution) is authenticated using something else than the
user principal associated with the HTTP request.

Ultimately we should probably solve the performance issue on the
repository layer by making path lookups (especially negative ones)
blazingly fast. They're already pretty good in Jackrabbit, but for my
use case I'd need about an order of magnitude more performance. With a
simple high-level cache I was able to achieve this.

BR,

Jukka Zitting


Re: Caching for ResourceResolver.map()

2009-06-10 Thread Carsten Ziegeler
Jukka Zitting wrote:
> Hi,
> 
> I have a case where we're doing lots of reverse mappings with the
> ResourceResolver.map() method and all the repository accesses caused
> by that are hurting performance. In general I'm not a big fan of extra
> caching, but in this case it seems like the only easy way to solve the
> problem.
> 
> Would it make sense to embed such caching into JcrResourceResolver2 or
> should it rather be a separate add-on layer? The former approach would
> avoid complexities with the HttpServletRequest argument affecting the
> caching, while the latter would make it easier to only apply the
> caching in limited cases where it's truly needed (though at the cost
> of the cache not being global).
> 
While caching in the resource resolver might be a good idea anyway,
perhaps caching one layer above might give even more benefit. Instead
of querying the resource resolver a lot, caching above the resolver
would even avoid querying the resolver.

Carsten

-- 
Carsten Ziegeler
cziege...@apache.org


Re: Caching for ResourceResolver.map()

2009-06-10 Thread Felix Meschberger
Hi,

Jukka Zitting schrieb:
> Hi,
> 
> I have a case where we're doing lots of reverse mappings with the
> ResourceResolver.map() method and all the repository accesses caused
> by that are hurting performance. In general I'm not a big fan of extra
> caching, but in this case it seems like the only easy way to solve the
> problem.
> 
> Would it make sense to embed such caching into JcrResourceResolver2 or
> should it rather be a separate add-on layer? The former approach would
> avoid complexities with the HttpServletRequest argument affecting the
> caching, while the latter would make it easier to only apply the
> caching in limited cases where it's truly needed (though at the cost
> of the cache not being global).

I think we should embed the caching rather deeply into the
JcrResourceResolver2 to leverage as much globally valid cache data as
possible.

What we might consider is doing per-user caches to leverage access
control of the repository.

And while implementing a cache for the map() methods, implementing a
cache for the resolve() methods would probably come in handy and almost
free, since both methods internally use the same resolveInternal() method.

Regards
Felix


Caching for ResourceResolver.map()

2009-06-10 Thread Jukka Zitting
Hi,

I have a case where we're doing lots of reverse mappings with the
ResourceResolver.map() method and all the repository accesses caused
by that are hurting performance. In general I'm not a big fan of extra
caching, but in this case it seems like the only easy way to solve the
problem.

Would it make sense to embed such caching into JcrResourceResolver2 or
should it rather be a separate add-on layer? The former approach would
avoid complexities with the HttpServletRequest argument affecting the
caching, while the latter would make it easier to only apply the
caching in limited cases where it's truly needed (though at the cost
of the cache not being global).

BR,

Jukka Zitting