On Wed, Jul 30, 2014 at 4:41 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 28.07.2014 17:21, stef...@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.
