On Fri, Mar 02, 2018 at 09:40:28PM +0000, Philip Martin wrote: > Philip Martin <phi...@codematters.co.uk> writes: > > > Again, 1.10 would be nearly twice as fast, but now rereading the authz > > removes most of that gain. > > I think I see the underlying problem: the authz code now incorporates a > cache based on the md5 checksum of the rules, so when the rules are > unchanged the cached value can be reused. This cache relies on the > caller being able to pass an svn_repos_t to svn_repos_authz_read3() and, > while svnserve does indeed pass such a pointer, mod_authz_svn is passing > NULL. That means mod_authz_svn does not take advantage of the new authz > cache. > > Stefan's pool patch helps, but I believe the authz rereading in > mod_authz_svn should be reverted from 1.10 unless we can make it take > advantage of the new authz cache.
Yes, the problem seems to be that mod_authz_svn has no way of storing per-connection state at present. The memory usage problem happened because it kept adding new access_conf objects to the per-connection pool since it has no way of knowing whether a previous request did already create the same object. We could store per-connection data by using members of r->conn, such as r->conn->id or r->notes to index into a cache of svn_repos_t stored in r->conn's pool. But where could we store a pointer to this cache so that it could be read back from the request structure r? Maybe we could use a global cache of svn_repos_t objects which live throughout the lifetime of mod_authz_svn and are shared across connections? It probably won't grow out of bounds as the number of repositories is relatively constant. But it's unclear how to ensure it stays in sync with the on-disk state. Hmm...