[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Authz perf regression 1.9 -> 1.10

From: Sam Toliman <ivansduck_at_gmail.com>
Date: Thu, 12 Sep 2019 14:30:45 +0300

>> (~3500k in my case)
> You have more than 3 million groups?
Misprinted. There are ~3500 groups in my case.

> The best thing you can do is prepare a patch, with tests, that
> implements your suggestions. Considering of course that correctness is
> more important than performance. :)
I will try to do it properly, in accordance with the algorithm described
above.

I have played with code base and ended up with minimal ad hoc diff required
to speed up svn_authz__parse() [1]. It's certainly not a patch candidate,
but can you please verify that this user filter doesn't break any implicit
contracts or semantic agreements in the auth logic (see
prepare_global_rights())?

It gave some performance improvements (best of 10 runs):
1.10:
time SVN_SSH='/tmp/svnserve1.10.sh' svn info svn+ssh://{server}/arc
>/dev/null
 real 0m0,501s
 user 0m0,009s
 sys 0m0,008s

1.10 with patch:
time SVN_SSH='/tmp/svnserve1.10_patch.sh' svn info svn+ssh://{server}/arc
>/dev/null
 real 0m0,134s
 user 0m0,001s
 sys 0m0,014s

 1.9:
time SVN_SSH='/tmp/svnserve1.9.sh' svn info svn+ssh://{server}/arc
>/dev/null
 real 0m0,114s
 user 0m0,001s
 sys 0m0,016s

Tests passed:
make check...
Summary of test results:
2415 tests PASSED
94 tests SKIPPED
57 tests XFAILED (1 WORK-IN-PROGRESS)
SUMMARY: All tests successful.

/tmp/svnserve*.sh content:
 #!/bin/bash
 {path_to_svnserve} --memory-cache-size 128 --compression 0 -t -r
/storage/rep/ --config-file=/storage/rep/svnserve.conf --log-file /dev/null

[1] diff applied to 1.10:

Index: subversion/libsvn_repos/authz_parse.c
===================================================================
--- subversion/libsvn_repos/authz_parse.c (revision 5646686)
+++ subversion/libsvn_repos/authz_parse.c (working copy)
@@ -133,6 +133,10 @@

      N.B.: The result pool is AUTHZ->pool. */
   apr_pool_t *parser_pool;
+
+ /* Stores tunnel_user to speed up ACL loading in tunnel mode.
+ For more info see ARCADIA-1621. */
+ const char *tunnel_user;
 } ctor_baton_t;

@@ -242,6 +246,11 @@
 static void
 prepare_global_rights(ctor_baton_t *cb, const char *user)
 {
+ if (cb->tunnel_user)
+ {
+ if (0 != strcmp(cb->tunnel_user, user))
+ return;
+ }
   authz_global_rights_t *gr = svn_hash_gets(cb->authz->user_rights, user);
   if (!gr)
     {
@@ -1298,10 +1307,11 @@
                  svn_stream_t *rules,
                  svn_stream_t *groups,
                  apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
+ apr_pool_t *scratch_pool,
+ const char *tunnel_user)
 {
   ctor_baton_t *const cb = create_ctor_baton(result_pool, scratch_pool);
-
+ cb->tunnel_user = tunnel_user;
   /*
    * Pass 1: Parse the authz file.
    */
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 5646686)
+++ subversion/include/svn_repos.h (working copy)
@@ -4170,7 +4170,8 @@
                       svn_boolean_t must_exist,
                       svn_repos_t *repos_hint,
                       apr_pool_t *result_pool,
- apr_pool_t *scratch_pool);
+ apr_pool_t *scratch_pool,
+ const char *tunnel_user);

 /**
  * Similar to svn_repos_authz_read3(), but with @a repos_hint set to @c
NULL.
Index: subversion/libsvn_repos/authz.c
===================================================================
--- subversion/libsvn_repos/authz.c (revision 5646686)
+++ subversion/libsvn_repos/authz.c (working copy)
@@ -1549,7 +1549,8 @@
            svn_boolean_t must_exist,
            svn_repos_t *repos_hint,
            apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
+ apr_pool_t *scratch_pool,
+ const char *tunnel_user)
 {
   svn_error_t* err = NULL;
   svn_stream_t *rules_stream = NULL;
@@ -1587,7 +1588,7 @@
           /* Parse the configuration(s) and construct the full authz model
            * from it. */
           err = svn_authz__parse(authz_p, rules_stream, groups_stream,
- item_pool, scratch_pool);
+ item_pool, scratch_pool, tunnel_user);
           if (err != SVN_NO_ERROR)
             {
               /* That pool would otherwise never get destroyed. */
@@ -1613,7 +1614,7 @@
        * it. */
       err = svn_error_quick_wrapf(svn_authz__parse(authz_p, rules_stream,
                                                    groups_stream,
- result_pool,
scratch_pool),
+ result_pool,
scratch_pool, tunnel_user),
                                   "Error while parsing authz file: '%s':",
                                   path);
     }
@@ -1634,13 +1635,14 @@
                       svn_boolean_t must_exist,
                       svn_repos_t *repos_hint,
                       apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
+ apr_pool_t *scratch_pool,
+ const char *tunnel_user)
 {
   svn_authz_t *authz = apr_pcalloc(result_pool, sizeof(*authz));
   authz->pool = result_pool;

   SVN_ERR(authz_read(&authz->full, &authz->authz_id, path, groups_path,
- must_exist, repos_hint, result_pool, scratch_pool));
+ must_exist, repos_hint, result_pool, scratch_pool,
tunnel_user));

   *authz_p = authz;
   return SVN_NO_ERROR;
@@ -1657,7 +1659,7 @@

   /* Parse the configuration and construct the full authz model from it. */
   SVN_ERR(svn_authz__parse(&authz->full, stream, groups_stream, pool,
- scratch_pool));
+ scratch_pool, NULL));

   svn_pool_destroy(scratch_pool);

Index: subversion/libsvn_repos/authz.h
===================================================================
--- subversion/libsvn_repos/authz.h (revision 5646686)
+++ subversion/libsvn_repos/authz.h (working copy)
@@ -304,7 +304,8 @@
                  svn_stream_t *rules,
                  svn_stream_t *groups,
                  apr_pool_t *result_pool,
- apr_pool_t *scratch_pool);
+ apr_pool_t *scratch_pool,
+ const char *tunnel_user);

 /* Reverse a STRING of length LEN in place. */
Index: subversion/libsvn_repos/deprecated.c
===================================================================
--- subversion/libsvn_repos/deprecated.c (revision 5646686)
+++ subversion/libsvn_repos/deprecated.c (working copy)
@@ -1282,7 +1282,7 @@
   apr_pool_t *scratch_pool = svn_pool_create(pool);
   svn_error_t *err = svn_repos_authz_read3(authz_p, path, groups_path,
                                            must_exist, NULL,
- pool, scratch_pool);
+ pool, scratch_pool, NULL);
   svn_pool_destroy(scratch_pool);

   return svn_error_trace(err);
Index: subversion/mod_authz_svn/mod_authz_svn.c
===================================================================
--- subversion/mod_authz_svn/mod_authz_svn.c (revision 5646686)
+++ subversion/mod_authz_svn/mod_authz_svn.c (working copy)
@@ -475,7 +475,7 @@
       svn_err = svn_repos_authz_read3(&access_conf, access_file,
                                       groups_file, TRUE, NULL,
                                       r->connection->pool,
- scratch_pool);
+ scratch_pool, NULL);

       if (svn_err)
         {
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 5646686)
+++ subversion/svnserve/serve.c (working copy)
@@ -294,7 +294,8 @@
 load_authz_config(repository_t *repository,
                   const char *repos_root,
                   svn_config_t *cfg,
- apr_pool_t *pool)
+ apr_pool_t *pool,
+ const char *tunnel_user)
 {
   const char *authzdb_path;
   const char *groupsdb_path;
@@ -323,7 +324,7 @@
       if (!err)
         err = svn_repos_authz_read3(&repository->authzdb, authzdb_path,
                                     groupsdb_path, TRUE, repository->repos,
- pool, pool);
+ pool, pool, tunnel_user);

       if (err)
         return svn_error_create(SVN_ERR_AUTHZ_INVALID_CONFIG, err, NULL);
@@ -3794,7 +3795,8 @@
            svn_repos__config_pool_t *config_pool,
            apr_hash_t *fs_config,
            apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
+ apr_pool_t *scratch_pool,
+ const char *tunnel_user)
 {
   const char *path, *full_path, *fs_path, *hooks_env;
   svn_stringbuf_t *url_buf;
@@ -3870,7 +3872,7 @@

   SVN_ERR(load_pwdb_config(repository, cfg, config_pool, result_pool));
   SVN_ERR(load_authz_config(repository, repository->repos_root, cfg,
- result_pool));
+ result_pool, tunnel_user));

   /* Should we use Cyrus SASL? */
   SVN_ERR(svn_config_get_bool(cfg, &sasl_requested,
@@ -4218,7 +4220,8 @@
                                        b->read_only, params->cfg,
                                        b->repository, params->config_pool,
                                        params->fs_config,
- conn_pool, scratch_pool),
+ conn_pool, scratch_pool,
+ b->client_info->tunnel_user),
                             b);
   if (!err)
     {
Index: tools/server-side/svnauthz.c
===================================================================
--- tools/server-side/svnauthz.c (revision 5646686)
+++ tools/server-side/svnauthz.c (working copy)
@@ -277,7 +277,7 @@
   /* Else */
   return svn_repos_authz_read3(authz, opt_state->authz_file,
                                opt_state->groups_file,
- TRUE, NULL, pool, pool);
+ TRUE, NULL, pool, pool, NULL);
 }

 static svn_error_t *
Received on 2019-09-12 13:31:04 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.