[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1614052 - in /subversion/branches/authzperf/subversion/libsvn_repos: authz.c authz_pool.c repos.h

From: Branko Čibej <brane_at_wandisco.com>
Date: Fri, 01 Aug 2014 13:23:16 +0200

On 01.08.2014 13:07, Stefan Fuhrmann wrote:
> On Wed, Jul 30, 2014 at 4:41 PM, Branko Čibej <brane_at_wandisco.com
> <mailto:brane_at_wandisco.com>> wrote:
>
> On 28.07.2014 17:21, stefan2_at_apache.org
> <mailto:stefan2_at_apache.org> wrote:
>> Author: stefan2
>> Date: Mon Jul 28 15:21:43 2014
>> New Revision: 1614052
>>
>> URL: http://svn.apache.org/r1614052
>> Log:
>> On the authzperf branch: Hide the svn_authz_t definition from
>> the headers again. This will allow us to later store prefix tree
>> data etc. in that structure.
>
> Stefan^2 , what can we do to make the authz_pool less intrusive?
> Making assumptions about what it stores seems like a really bad
> idea. Can the pool just store an opaque pointer, or a
> forward-declared type, and /not/ assume it's an svn_config_t (or
> svn_authz_t, as it was before your change)? Does it need the size
> of the structure, or anything like that?
>
>
> authz_pool is not meant to be intrusive. The basic idea has been
> to cache
>
> * the "ingredients" (constructor parameters) we need to create an authz
> * the authz instance itself
>
> Right now, it does not actually cache much itself, it mainly holds a
> structure
> that keeps counted references (handed out by config_pool) to the 1 or 2
> svn_config_t* instances (in case groups come from a separate config)
> needed to construct the authz.
>
> Once we don't use svn_config_t * as an intermediate anymore, we can
> simply store an opaque pointer to the new model. At that point, authz_pool
> might be merged with the generic object_pool code as the remaining
> configuration files hardly benefit from caching making config_pool
> obsolete.
>
>
> I'm still not clear on why we need the authz_pool in the first
> place. The documentation and code are, to put it mildly, rather dense.
>
>
> Having authz_pool is not functionally required. With the upcoming authz
> parser improvements, it may not even be a performance improvement
> for file-based authz. Calculating the sha1 checksum on the file before
> actually parsing it may then simply be too much of an overhead.
>
> For completely repo-based authz, it eliminates the initial parsing phase
> with virtually no overhead for the out-of-date check. This cuts down on
> access latency for large authz. That's the sole purpose of authz_pool.
>
> If it should turn out to not be worth the hassle, I completely fine with
> eventually dropping it entirely.

Ack, thanks for the explanation!

I'm currently working on the improved parser and an in-memory
representation that should make generating the per-user filtered rules
much easier and faster. I'm not quite ready to commit right now, since
I'm in the middle of the second iteration of the in-memory struct design. :)

I'm doing this in new source files (will be authz_parse.c and
authz-test.c) so changing authz.c is fine for now.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane_at_wandisco.com
Received on 2014-08-01 13:23:33 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.