subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rhuij...@apache.org
Subject svn commit: r1673170 - in /subversion/trunk/subversion: include/ libsvn_fs/ libsvn_fs_base/ libsvn_fs_fs/ libsvn_fs_x/ libsvn_ra_local/ libsvn_repos/ mod_dav_svn/ svnserve/ tests/libsvn_ra/
Date Mon, 13 Apr 2015 12:23:21 GMT
Author: rhuijben
Date: Mon Apr 13 12:23:20 2015
New Revision: 1673170

URL: http://svn.apache.org/r1673170
Log:
Add an fs layer api that allows obtaining just a boolean indicating whether
properties exist on a node, instead of always obtaining the properties and
checking their count.

This is by far the most expensive operation on 'svn ls -v' in Subversion <=
1.8.x on huge directories as it requires fetching much 'new' data, and has
the risk of trashing the node cache.

r1673153 made new 'svn' clients stop asking for this information for this
scenario but existing clients do ask and so will most likely many third
party clients (confirmed for TortoiseSVN), will keep asking for this
information.

This function introduces the FS api and updates callers, but doesn't provide
optimized implementations yet, so the result is that this doesn't change
runtime behaviour yet, but just moves the implementation into the fs layer.

I hope this patch will be accepted for 1.9.0 to allow further improvements
in later patches, potentially after 1.9.0.

* subversion/include/svn_fs.h
  (svn_fs_node_has_props): New function.

* subversion/libsvn_fs/fs-loader.c
  (svn_fs_node_has_props): New function.

* subversion/libsvn_fs/fs-loader.h
  (root_vtable_t): Add node_has_props.

* subversion/libsvn_fs_base/tree.c
  (base_node_has_props): New function.
  (root_vtable): Add function.

* subversion/libsvn_fs_fs/tree.c
  (fs_node_has_props): New function.
  (root_vtable): Add function.

* subversion/libsvn_fs_x/tree.c
  (x_node_has_props): New function.
  (root_vtable): Add function.

* subversion/libsvn_ra_local/ra_plugin.c
  (svn_ra_local__get_dir): Use new optimized fs call. Rename subpool
    to iterpool.

* subversion/libsvn_repos/repos.c
  (svn_repos_stat): Use new optimized fs call.

* subversion/mod_dav_svn/liveprops.c
  (insert_prop_internal): Use optimized code for svn clients.

* subversion/svnserve/serve.c
  (get_dir): Use optimized fs code.

* subversion/tests/libsvn_ra/ra-test.c
  (ra_list_has_props): New function.
  (test_funcs): Add ra_list_has_props.

Modified:
    subversion/trunk/subversion/include/svn_fs.h
    subversion/trunk/subversion/libsvn_fs/fs-loader.c
    subversion/trunk/subversion/libsvn_fs/fs-loader.h
    subversion/trunk/subversion/libsvn_fs_base/tree.c
    subversion/trunk/subversion/libsvn_fs_fs/tree.c
    subversion/trunk/subversion/libsvn_fs_x/tree.c
    subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
    subversion/trunk/subversion/libsvn_repos/repos.c
    subversion/trunk/subversion/mod_dav_svn/liveprops.c
    subversion/trunk/subversion/svnserve/serve.c
    subversion/trunk/subversion/tests/libsvn_ra/ra-test.c

Modified: subversion/trunk/subversion/include/svn_fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_fs.h?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_fs.h (original)
+++ subversion/trunk/subversion/include/svn_fs.h Mon Apr 13 12:23:20 2015
@@ -1823,6 +1823,18 @@ svn_fs_node_proplist(apr_hash_t **table_
                      const char *path,
                      apr_pool_t *pool);
 
+/** Set @a *has_props to TRUE if the node @a path in @a root has properties
+ * and to FALSE if it doesn't have properties. Perform temporary allocations
+ * in @a scratch_pool.
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_fs_node_has_props(svn_boolean_t *has_props,
+                      svn_fs_root_t *root,
+                      const char *path,
+                      apr_pool_t *scratch_pool);
+
 
 /** Change a node's property's value, or add/delete a property.
  *

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Mon Apr 13 12:23:20 2015
@@ -1196,6 +1196,16 @@ svn_fs_node_proplist(apr_hash_t **table_
 }
 
 svn_error_t *
+svn_fs_node_has_props(svn_boolean_t *has_props,
+                      svn_fs_root_t *root,
+                      const char *path,
+                      apr_pool_t *scratch_pool)
+{
+  return svn_error_trace(root->vtable->node_has_props(has_props, root, path,
+                                                      scratch_pool));
+}
+
+svn_error_t *
 svn_fs_change_node_prop(svn_fs_root_t *root, const char *path,
                         const char *name, const svn_string_t *value,
                         apr_pool_t *pool)

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.h?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.h (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.h Mon Apr 13 12:23:20 2015
@@ -346,6 +346,8 @@ typedef struct root_vtable_t
                             apr_pool_t *pool);
   svn_error_t *(*node_proplist)(apr_hash_t **table_p, svn_fs_root_t *root,
                                 const char *path, apr_pool_t *pool);
+  svn_error_t *(*node_has_props)(svn_boolean_t *has_props, svn_fs_root_t *root,
+                                 const char *path, apr_pool_t *scratch_pool);
   svn_error_t *(*change_node_prop)(svn_fs_root_t *root, const char *path,
                                    const char *name,
                                    const svn_string_t *value,

Modified: subversion/trunk/subversion/libsvn_fs_base/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_base/tree.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_base/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_base/tree.c Mon Apr 13 12:23:20 2015
@@ -1285,6 +1285,21 @@ base_node_proplist(apr_hash_t **table_p,
   return SVN_NO_ERROR;
 }
 
+static svn_error_t *
+base_node_has_props(svn_boolean_t *has_props,
+                    svn_fs_root_t *root,
+                    const char *path,
+                    apr_pool_t *scratch_pool)
+{
+  apr_hash_t *props;
+
+  SVN_ERR(base_node_proplist(&props, root, path, scratch_pool));
+
+  *has_props = (0 < apr_hash_count(props));
+
+  return SVN_NO_ERROR;
+}
+
 
 struct change_node_prop_args {
   svn_fs_root_t *root;
@@ -5492,6 +5507,7 @@ static root_vtable_t root_vtable = {
   base_closest_copy,
   base_node_prop,
   base_node_proplist,
+  base_node_has_props,
   base_change_node_prop,
   base_props_changed,
   base_dir_entries,

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Mon Apr 13 12:23:20 2015
@@ -1515,6 +1515,20 @@ fs_node_proplist(apr_hash_t **table_p,
   return SVN_NO_ERROR;
 }
 
+static svn_error_t *
+fs_node_has_props(svn_boolean_t *has_props,
+                  svn_fs_root_t *root,
+                  const char *path,
+                  apr_pool_t *scratch_pool)
+{
+  apr_hash_t *props;
+
+  SVN_ERR(fs_node_proplist(&props, root, path, scratch_pool));
+
+  *has_props = (0 < apr_hash_count(props));
+
+  return SVN_NO_ERROR;
+}
 
 static svn_error_t *
 increment_mergeinfo_up_tree(parent_path_t *pp,
@@ -4238,6 +4252,7 @@ static root_vtable_t root_vtable = {
   fs_closest_copy,
   fs_node_prop,
   fs_node_proplist,
+  fs_node_has_props,
   fs_change_node_prop,
   fs_props_changed,
   fs_dir_entries,

Modified: subversion/trunk/subversion/libsvn_fs_x/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/tree.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/tree.c Mon Apr 13 12:23:20 2015
@@ -1500,6 +1500,20 @@ x_node_proplist(apr_hash_t **table_p,
   return SVN_NO_ERROR;
 }
 
+static svn_error_t *
+x_node_has_props(svn_boolean_t *has_props,
+                 svn_fs_root_t *root,
+                 const char *path,
+                 apr_pool_t *scratch_pool)
+{
+  apr_hash_t *props;
+
+  SVN_ERR(x_node_proplist(&props, root, path, scratch_pool));
+
+  *has_props = (0 < apr_hash_count(props));
+
+  return SVN_NO_ERROR;
+}
 
 static svn_error_t *
 increment_mergeinfo_up_tree(parent_path_t *pp,
@@ -4214,6 +4228,7 @@ static root_vtable_t root_vtable = {
   x_closest_copy,
   x_node_prop,
   x_node_proplist,
+  x_node_has_props,
   x_change_node_prop,
   x_props_changed,
   x_dir_entries,

Modified: subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c (original)
+++ subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c Mon Apr 13 12:23:20 2015
@@ -1309,7 +1309,6 @@ svn_ra_local__get_dir(svn_ra_session_t *
   apr_hash_t *entries;
   apr_hash_index_t *hi;
   svn_ra_local__session_baton_t *sess = session->priv;
-  apr_pool_t *subpool;
   const char *abs_path = svn_fspath__join(sess->fs_path->data, path, pool);
 
   /* Open the revision's root. */
@@ -1325,29 +1324,28 @@ svn_ra_local__get_dir(svn_ra_session_t *
 
   if (dirents)
     {
+      apr_pool_t *iterpool = svn_pool_create(pool);
       /* Get the dir's entries. */
       SVN_ERR(svn_fs_dir_entries(&entries, root, abs_path, pool));
 
       /* Loop over the fs dirents, and build a hash of general
          svn_dirent_t's. */
       *dirents = apr_hash_make(pool);
-      subpool = svn_pool_create(pool);
       for (hi = apr_hash_first(pool, entries); hi; hi = apr_hash_next(hi))
         {
           const void *key;
           void *val;
-          apr_hash_t *prophash;
           const char *datestring, *entryname, *fullpath;
           svn_fs_dirent_t *fs_entry;
           svn_dirent_t *entry = svn_dirent_create(pool);
 
-          svn_pool_clear(subpool);
+          svn_pool_clear(iterpool);
 
           apr_hash_this(hi, &key, NULL, &val);
           entryname = (const char *) key;
           fs_entry = (svn_fs_dirent_t *) val;
 
-          fullpath = svn_dirent_join(abs_path, entryname, subpool);
+          fullpath = svn_dirent_join(abs_path, entryname, iterpool);
 
           if (dirent_fields & SVN_DIRENT_KIND)
             {
@@ -1362,15 +1360,15 @@ svn_ra_local__get_dir(svn_ra_session_t *
                 entry->size = 0;
               else
                 SVN_ERR(svn_fs_file_length(&(entry->size), root,
-                                           fullpath, subpool));
+                                           fullpath, iterpool));
             }
 
           if (dirent_fields & SVN_DIRENT_HAS_PROPS)
             {
               /* has_props? */
-              SVN_ERR(svn_fs_node_proplist(&prophash, root, fullpath,
-                                           subpool));
-              entry->has_props = (apr_hash_count(prophash) != 0);
+              SVN_ERR(svn_fs_node_has_props(&entry->has_props,
+                                            root, fullpath,
+                                            iterpool));
             }
 
           if ((dirent_fields & SVN_DIRENT_TIME)
@@ -1381,7 +1379,7 @@ svn_ra_local__get_dir(svn_ra_session_t *
               SVN_ERR(svn_repos_get_committed_info(&(entry->created_rev),
                                                    &datestring,
                                                    &(entry->last_author),
-                                                   root, fullpath, subpool));
+                                                   root, fullpath, iterpool));
               if (datestring)
                 SVN_ERR(svn_time_from_cstring(&(entry->time), datestring,
                                               pool));
@@ -1392,7 +1390,7 @@ svn_ra_local__get_dir(svn_ra_session_t *
           /* Store. */
           svn_hash_sets(*dirents, entryname, entry);
         }
-      svn_pool_destroy(subpool);
+      svn_pool_destroy(iterpool);
     }
 
   /* Handle props if requested. */

Modified: subversion/trunk/subversion/libsvn_repos/repos.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/repos.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/repos.c (original)
+++ subversion/trunk/subversion/libsvn_repos/repos.c Mon Apr 13 12:23:20 2015
@@ -2083,9 +2083,7 @@ svn_repos_stat(svn_dirent_t **dirent,
   if (kind == svn_node_file)
     SVN_ERR(svn_fs_file_length(&(ent->size), root, path, pool));
 
-  SVN_ERR(svn_fs_node_proplist(&prophash, root, path, pool));
-  if (apr_hash_count(prophash) > 0)
-    ent->has_props = TRUE;
+  SVN_ERR(svn_fs_node_has_props(&ent->has_props, root, path, pool));
 
   SVN_ERR(svn_repos_get_committed_info(&(ent->created_rev),
                                        &datestring,

Modified: subversion/trunk/subversion/mod_dav_svn/liveprops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/liveprops.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/liveprops.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/liveprops.c Mon Apr 13 12:23:20 2015
@@ -787,30 +787,58 @@ insert_prop_internal(const dav_resource
 
     case SVN_PROPID_deadprop_count:
       {
-        unsigned int propcount;
-        apr_hash_t *proplist;
-
         if (resource->type != DAV_RESOURCE_TYPE_REGULAR)
           return DAV_PROP_INSERT_NOTSUPP;
 
-        serr = svn_fs_node_proplist(&proplist,
-                                    resource->info->root.root,
-                                    resource->info->repos_path, scratch_pool);
-        if (serr != NULL)
+        if (resource->info->repos->is_svn_client)
           {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, serr->apr_err,
-                          resource->info->r,
-                          "Can't fetch proplist of '%s': "
-                          "%s",
-                          resource->info->repos_path,
-                          serr->message);
-            svn_error_clear(serr);
-            value = error_value;
-            break;
+            svn_boolean_t has_props;
+            /* Retrieving the actual properties is quite expensive while
+               svn clients only want to know if there are properties, and
+               in many cases aren't interested at all (see r1673153) */
+            serr = svn_fs_node_has_props(&has_props,
+                                         resource->info->root.root,
+                                         resource->info->repos_path,
+                                         scratch_pool);
+
+            if (serr != NULL)
+              {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, serr->apr_err,
+                              resource->info->r,
+                              "Can't fetch has props of '%s': "
+                              "%s",
+                              resource->info->repos_path,
+                              serr->message);
+                svn_error_clear(serr);
+                value = error_value;
+                break;
+              }
+
+            value = has_props ? "99" /* Magic (undocumented) value */ : "0";
           }
+        else
+          {
+            unsigned int propcount;
+            apr_hash_t *proplist;
+            serr = svn_fs_node_proplist(&proplist,
+                                        resource->info->root.root,
+                                        resource->info->repos_path, scratch_pool);
+            if (serr != NULL)
+              {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, serr->apr_err,
+                              resource->info->r,
+                              "Can't fetch proplist of '%s': "
+                              "%s",
+                              resource->info->repos_path,
+                              serr->message);
+                svn_error_clear(serr);
+                value = error_value;
+                break;
+              }
 
-        propcount = apr_hash_count(proplist);
-        value = apr_psprintf(scratch_pool, "%u", propcount);
+            propcount = apr_hash_count(proplist);
+            value = apr_psprintf(scratch_pool, "%u", propcount);
+          }
         break;
       }
 

Modified: subversion/trunk/subversion/svnserve/serve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/serve.c (original)
+++ subversion/trunk/subversion/svnserve/serve.c Mon Apr 13 12:23:20 2015
@@ -1779,12 +1779,9 @@ static svn_error_t *get_dir(svn_ra_svn_c
 
           if (dirent_fields & SVN_DIRENT_HAS_PROPS)
             {
-              apr_hash_t *file_props;
-
               /* has_props */
-              SVN_CMD_ERR(svn_fs_node_proplist(&file_props, root, file_path,
+              SVN_CMD_ERR(svn_fs_node_has_props(&has_props, root, file_path,
                                                subpool));
-              has_props = (apr_hash_count(file_props) > 0);
             }
 
           if ((dirent_fields & SVN_DIRENT_LAST_AUTHOR)

Modified: subversion/trunk/subversion/tests/libsvn_ra/ra-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_ra/ra-test.c?rev=1673170&r1=1673169&r2=1673170&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_ra/ra-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_ra/ra-test.c Mon Apr 13 12:23:20 2015
@@ -1440,6 +1440,85 @@ errors_from_callbacks(const svn_test_opt
   return SVN_NO_ERROR;
 }
 
+static svn_error_t *
+ra_list_has_props(const svn_test_opts_t *opts,
+                  apr_pool_t *pool)
+{
+  svn_ra_session_t *ra_session;
+  const svn_delta_editor_t *editor;
+  apr_pool_t *iterpool = svn_pool_create(pool);
+  int i;
+  void *edit_baton;
+  const char *trunk_url;
+
+  SVN_ERR(make_and_open_repos(&ra_session, "ra_list_has_props",
+                              opts, pool));
+
+  SVN_ERR(svn_ra_get_commit_editor3(ra_session, &editor, &edit_baton,
+                                    apr_hash_make(pool), NULL,
+                                    NULL, NULL, FALSE, iterpool));
+
+  /* Create initial layout*/
+  {
+    void *root_baton;
+    void *dir_baton;
+
+    SVN_ERR(editor->open_root(edit_baton, 0, pool, &root_baton));
+    SVN_ERR(editor->add_directory("trunk", root_baton, NULL, SVN_INVALID_REVNUM,
+                                  iterpool, &dir_baton));
+    SVN_ERR(editor->close_directory(dir_baton, iterpool));
+    SVN_ERR(editor->add_directory("tags", root_baton, NULL, SVN_INVALID_REVNUM,
+                                  iterpool, &dir_baton));
+    SVN_ERR(editor->close_directory(dir_baton, iterpool));
+    SVN_ERR(editor->close_directory(root_baton, iterpool));
+    SVN_ERR(editor->close_edit(edit_baton, iterpool));
+  }
+
+  SVN_ERR(svn_ra_get_repos_root2(ra_session, &trunk_url, pool));
+  trunk_url = svn_path_url_add_component2(trunk_url, "trunk", pool);
+
+  /* Create a few tags. Using a value like 8000 will take too long for a normal
+     testrun, but produces more realistic problems */
+  for (i = 0; i < 50; i++)
+    {
+      void *root_baton;
+      void *tags_baton;
+      void *dir_baton;
+
+      svn_pool_clear(iterpool);
+
+      SVN_DBG(("r%d", i+1));
+
+      SVN_ERR(svn_ra_get_commit_editor3(ra_session, &editor, &edit_baton,
+                                        apr_hash_make(pool), NULL,
+                                        NULL, NULL, FALSE, iterpool));
+
+      SVN_ERR(editor->open_root(edit_baton, i+1, pool, &root_baton));
+      SVN_ERR(editor->open_directory("tags", root_baton, i+1, iterpool,
+                                     &tags_baton));
+      SVN_ERR(editor->add_directory(apr_psprintf(iterpool, "tags/T%05d", i+1),
+                                    tags_baton, trunk_url, 1, iterpool,
+                                    &dir_baton));
+
+      SVN_ERR(editor->close_directory(dir_baton, iterpool));
+      SVN_ERR(editor->close_directory(tags_baton, iterpool));
+      SVN_ERR(editor->close_directory(root_baton, iterpool));
+      SVN_ERR(editor->close_edit(edit_baton, iterpool));
+    }
+
+  {
+    apr_hash_t *dirents;
+    svn_revnum_t fetched_rev;
+    apr_hash_t *props;
+
+    SVN_ERR(svn_ra_get_dir2(ra_session, &dirents, &fetched_rev, &props,
+                            "tags", SVN_INVALID_REVNUM,
+                            SVN_DIRENT_ALL, pool));
+  }
+
+  return SVN_NO_ERROR;
+}
+
 
 /* The test table.  */
 
@@ -1468,6 +1547,8 @@ static struct svn_test_descriptor_t test
                        "check how ra functions handle bad revisions"),
     SVN_TEST_OPTS_PASS(errors_from_callbacks,
                        "check how ra layers handle errors from callbacks"),
+    SVN_TEST_OPTS_PASS(ra_list_has_props,
+                       "check list has_props performance"),
     SVN_TEST_NULL
   };
 



Mime
View raw message