On Wed, Jul 30, 2014 at 4:41 PM, Branko Čibej <brane_at_wandisco.com> wrote:
> On 28.07.2014 17:21, 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.
>
>
> 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
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.
-- Stefan^2.
Received on 2014-08-01 13:08:04 CEST