Forum OpenACS Development: Re: Permission system in needs of revisit?

Collapse
Posted by Malte Sussdorff on
Heh, revisiting the ns_cache statistics before posting helps. One of the large benefits of not using util_memoize for permissions is that util_memoize will not run out of entries. This being said I dug a little bit deeper into permission::permission_p and got confused:

a) We could probably change the util_memoize usage to a db_cache_pool caching, though we would probably run into the same issue as with util_memoize.

b) Looking at permission::permission_p_not_cached I see this:

    # We have a thread-local cache here
    global permission__permission_p__cache
    if { ![info exists permission__permission_p__cache($party_id,$object_id,$privilege)] } {
        set permission__permission_p__cache($party_id,$object_id,$privilege) [db_0or1row select_permission_p {}]
    }
    return $permission__permission_p__cache($party_id,$object_id,$privilege)

How much sense does the thread level cache make if we have caching using util_memoize or a different caching mechanism?  I can only think that this acts as a protection against permissions changes during execution of the script, which i guess is unwanted. But if that is the case, shouldn't this script level caching happen in permission::permission_p and before the util_memoize / permission_cache_pool caching?

Additionally, reading up on https://openacs.org/doc/current/programming-with-aolserver.html, shouldn't this better be called "script-local" cache as the array is destroyed once the script has run through (or am I missing something here)?

Collapse
Posted by Gustaf Neumann on
The array permission__permission_p__cache just prevents that - during a single request - the same permission is only looked-up once from the database. This double-lookup-prevention happens no matter what other caching options are used. I think, this is a sensible behavior. Yes, the array is deleted in the thread after the request.

There is a small bug in permission::permission_p_not_cached: the unused non-positiional argument "-no_cache" should be most probably removed.

Collapse
Posted by Malte Sussdorff on
Yes, this lookup prevention is great, which is why I would put it outside the "not_cached" version and put it into permission::permission_p, because otherwise we hit the util_memoize cache more often than we need to. And my assumption is that the util_memoize lookup is slower than going for the array.

In my understanding the first thing permission::permission_p should do is to look up the array and if it finds an entry return that value, skipping all other checks and procedure calls. Does this make sense ?