subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fuhrmann <>
Subject Re: svn commit: r1614052 - in /subversion/branches/authzperf/subversion/libsvn_repos: authz.c authz_pool.c repos.h
Date Fri, 01 Aug 2014 11:07:36 GMT
On Wed, Jul 30, 2014 at 4:41 PM, Branko ─îibej <> wrote:

>  On 28.07.2014 17:21, wrote:
> Author: stefan2
> Date: Mon Jul 28 15:21:43 2014
> New Revision: 1614052
> URL:
> 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.
> Stefan2, 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
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.

-- Stefan^2.

View raw message