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