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

[PATCH] Issue 2151: 'svn ls' is slow over ra_dav (r3)

From: Kazutoshi Satoda <k_satoda_at_f2.dion.ne.jp>
Date: 2006-02-17 01:10:05 CET

Hi.

This is a revised patch for issue 2151 that speeds up 'svn ls' over
ra_dav. This is based on Jean-Marc Godbout's one(r2) for trunk@15946
and updated to trunk@18486 resolving some conflicts.
See http://svn.haxx.se/dev/archive-2005-09/0009.shtml

In my easy test with a directory that contains 800 files, this patch
improved 'svn ls' speed very much. Actually, from 1m~1m30s to 5s~30s.

I strongly wish that this patch is applied to 1.4.

Following log message is almost same, except removing dirent_props[]
that is no longer used.

[[[
Fix for issue 2151 "'svn ls' is slow over ra_dav"

This patch implements a solution to issue 2151.
We now only request the needed props in the PROPFIND
for server listings. 'svn ls' is now noticably faster.
In most cases 'ls' takes about half the time and half
the bandwidth - In some case even better results.

For backward compatibility, before doing the long PROPFIND
we make another simpler PROPFIND to see if the server
supports the new type of request (supports the
deadprop-count prop). If it does we use the new scheme
and performance is improved - If not then we use the old
scheme and the slowness persists.

   To summarise: only patched servers and patched clients
have improved speed. Mismatched configurations (old client
or server) are not improved but still work. Regular dav
clients are still slow.

Patch by: Jean-Marc Godbout <jean-marc@computrad.com>

Suggested by: Erik Scrafford <erik@scrafford.org>
Ben Collins-Sussman <sussman@collab.net>
Everyone else who contributed to issue 2151

* subversion/mod_dav_svn/liveprops.c
(SVN_PROPID_deadprop_count): Added a PROPID to support the new prop
(dav_svn_props[]): Added the deadprop-count prop to the list of
supported props.
(dav_svn_insert_prop): Returns the number of deadprops
found in the fs for that file.

   * subversion/libsvn_ra_dav/ra_dav.h
(SVN_RA_DAV__PROP_DEADPROP_COUNT): Added the prop name for deadprop
(ELEM_deadprop_count): Added the prop to the list of props

* subversion/libsvn_ra_dav/props.c
(elem_definition, propfind_elements): Added ELEM_deadprop_count
to the list of propfind props

* subversion/libsvn_ra_dav/fetch.c
(deadprop_count_support_props[]): New array, list of props to
get in a root PROPFIND
(svn_ra_dav__get_dir): check if server supports 'deadprop-count',
and if so, request fewer properties
]]]

--
k_satoda

Index: subversion/mod_dav_svn/liveprops.c
===================================================================
--- subversion/mod_dav_svn/liveprops.c (revision 18486)
+++ subversion/mod_dav_svn/liveprops.c (working copy)
@@ -67,7 +67,8 @@
 enum {
   SVN_PROPID_baseline_relative_path = 1,
   SVN_PROPID_md5_checksum,
- SVN_PROPID_repository_uuid
+ SVN_PROPID_repository_uuid,
+ SVN_PROPID_deadprop_count
 };
 
 static const dav_liveprop_spec dav_svn_props[] =
@@ -96,6 +97,7 @@
   SVN_RO_SVN_PROP(baseline_relative_path, baseline-relative-path),
   SVN_RO_SVN_PROP(md5_checksum, md5-checksum),
   SVN_RO_SVN_PROP(repository_uuid, repository-uuid),
+ SVN_RO_SVN_PROP(deadprop_count, deadprop-count),
 
   { 0 } /* sentinel */
 };
@@ -563,6 +565,30 @@
         }
       break;
 
+ 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, p);
+ if (serr != NULL)
+ {
+ /* ### what to do? */
+ svn_error_clear(serr);
+ value = "###error###";
+ break;
+ }
+
+ propcount = apr_hash_count(proplist);
+ value = apr_psprintf(p, "%u", propcount);
+ break;
+ }
+
     default:
       /* ### what the heck was this property? */
       return DAV_PROP_INSERT_NOTDEF;
Index: subversion/libsvn_ra_dav/ra_dav.h
===================================================================
--- subversion/libsvn_ra_dav/ra_dav.h (revision 18486)
+++ subversion/libsvn_ra_dav/ra_dav.h (working copy)
@@ -362,6 +362,7 @@
 
 #define SVN_RA_DAV__PROP_REPOSITORY_UUID SVN_DAV_PROP_NS_DAV "repository-uuid"
 
+#define SVN_RA_DAV__PROP_DEADPROP_COUNT SVN_DAV_PROP_NS_DAV "deadprop-count"
 
 typedef struct {
   /* what is the URL for this resource */
@@ -709,7 +710,8 @@
   ELEM_change_file_prop,
   ELEM_change_dir_prop,
   ELEM_close_file,
- ELEM_close_directory
+ ELEM_close_directory,
+ ELEM_deadprop_count
 };
 
 /* ### docco */
Index: subversion/libsvn_ra_dav/props.c
===================================================================
--- subversion/libsvn_ra_dav/props.c (revision 18486)
+++ subversion/libsvn_ra_dav/props.c (working copy)
@@ -111,6 +111,7 @@
   { ELEM_baseline_relpath, SVN_RA_DAV__PROP_BASELINE_RELPATH, 1 },
   { ELEM_md5_checksum, SVN_RA_DAV__PROP_MD5_CHECKSUM, 1 },
   { ELEM_repository_uuid, SVN_RA_DAV__PROP_REPOSITORY_UUID, 1 },
+ { ELEM_deadprop_count, SVN_RA_DAV__PROP_DEADPROP_COUNT, 1 },
   { 0 }
 };
 
@@ -147,6 +148,8 @@
     SVN_RA_DAV__XML_CDATA },
   { SVN_DAV_PROP_NS_DAV, "repository-uuid", ELEM_repository_uuid,
     SVN_RA_DAV__XML_CDATA },
+ { SVN_DAV_PROP_NS_DAV, "deadprop-count", ELEM_deadprop_count,
+ SVN_RA_DAV__XML_CDATA },
 
   /* Unknowns */
   { "", "", ELEM_unknown, SVN_RA_DAV__XML_COLLECT },
Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 18486)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -922,6 +922,13 @@
   return SVN_NO_ERROR;
 }
 
+/* the property we need to fetch to see if the server we are
+ connected to supports the deadprop-count property. */
+static const ne_propname deadprop_count_support_props[] =
+{
+ { SVN_DAV_PROP_NS_DAV, "deadprop-count" },
+ { NULL }
+};
 
 svn_error_t *svn_ra_dav__get_dir(svn_ra_session_t *session,
                                  const char *path,
@@ -937,6 +944,7 @@
   apr_hash_t *resources;
   const char *final_url;
   apr_size_t final_url_n_components;
+ svn_boolean_t supports_deadprop_count;
   svn_ra_dav__session_t *ras = session->priv;
   const char *url = svn_path_url_add_component(ras->url->data, path, pool);
 
@@ -964,13 +972,39 @@
         *fetched_rev = got_rev;
     }
 
+ /* For issue 2151:
+ See if we are dealing with a server that understand the
+ deadprop-count prop. If it doesn't we'll need to do an
+ allprop PROPFIND - If it does, we'll execute a more
+ targetted PROPFIND.
+ */
+
+ {
+ const svn_string_t *deadprop_count;
+
+ SVN_ERR( svn_ra_dav__get_props_resource(&rsrc, ras->sess,
+ final_url, NULL,
+ deadprop_count_support_props,
+ pool) );
+
+ deadprop_count = apr_hash_get(rsrc->propset,
+ SVN_RA_DAV__PROP_DEADPROP_COUNT,
+ APR_HASH_KEY_STRING);
+
+ if (deadprop_count == NULL)
+ supports_deadprop_count = FALSE;
+ else
+ supports_deadprop_count = TRUE;
+ }
+
   if (dirents)
     {
       ne_propname *which_props;
 
       /* if we didn't ask for the has_props field, we can get individual
          properties. */
- if ((SVN_DIRENT_HAS_PROPS & dirent_fields) == 0)
+ if ((SVN_DIRENT_HAS_PROPS & dirent_fields) == 0
+ || supports_deadprop_count)
         {
           int num_props = 1; /* start with one for the final NULL */
 
@@ -980,6 +1014,9 @@
           if (dirent_fields & SVN_DIRENT_SIZE)
             ++num_props;
 
+ if (dirent_fields & SVN_DIRENT_HAS_PROPS)
+ ++num_props;
+
           if (dirent_fields & SVN_DIRENT_CREATED_REV)
             ++num_props;
 
@@ -1012,6 +1049,12 @@
               which_props[num_props--].name = "getcontentlength";
             }
 
+ if (dirent_fields & SVN_DIRENT_HAS_PROPS)
+ {
+ which_props[num_props].nspace = SVN_DAV_PROP_NS_DAV;
+ which_props[num_props--].name = "deadprop-count";
+ }
+
           if (dirent_fields & SVN_DIRENT_CREATED_REV)
             {
               which_props[num_props].nspace = "DAV:";
@@ -1062,6 +1105,7 @@
           const svn_string_t *propval;
           apr_hash_index_t *h;
           svn_dirent_t *entry;
+ apr_int64_t prop_count;
           
           apr_hash_this(hi, &key, NULL, &val);
           childname = key;
@@ -1099,20 +1143,50 @@
             {
               /* does this resource contain any 'svn' or 'custom' properties,
                  i.e. ones actually created and set by the user? */
- for (h = apr_hash_first(pool, resource->propset);
- h; h = apr_hash_next(h))
+ if (supports_deadprop_count == TRUE)
                 {
- const void *kkey;
- void *vval;
- apr_hash_this(h, &kkey, NULL, &vval);
-
- if (strncmp((const char *)kkey, SVN_DAV_PROP_NS_CUSTOM,
- sizeof(SVN_DAV_PROP_NS_CUSTOM) - 1) == 0)
- entry->has_props = TRUE;
-
- else if (strncmp((const char *)kkey, SVN_DAV_PROP_NS_SVN,
- sizeof(SVN_DAV_PROP_NS_SVN) - 1) == 0)
- entry->has_props = TRUE;
+ propval = apr_hash_get(resource->propset,
+ SVN_RA_DAV__PROP_DEADPROP_COUNT,
+ APR_HASH_KEY_STRING);
+
+ if (propval == NULL)
+ {
+ /* we thought that the server supported the
+ deadprop-count property. apparently not. */
+ return svn_error_create(SVN_ERR_INCOMPLETE_DATA, NULL,
+ _("Server response missing the "
+ "expected deadprop-count "
+ "property")
+ );
+ }
+ else
+ {
+ prop_count = svn__atoui64(propval->data);
+ if (prop_count == 0)
+ entry->has_props = FALSE;
+ else
+ entry->has_props = TRUE;
+ }
+ }
+ else
+ {
+ /* The server doesn't support the deadprop_count prop,
+ fallback */
+ for (h = apr_hash_first(pool, resource->propset);
+ h; h = apr_hash_next(h))
+ {
+ const void *kkey;
+ void *vval;
+ apr_hash_this(h, &kkey, NULL, &vval);
+
+ if (strncmp((const char *)kkey, SVN_DAV_PROP_NS_CUSTOM,
+ sizeof(SVN_DAV_PROP_NS_CUSTOM) - 1) == 0)
+ entry->has_props = TRUE;
+
+ else if (strncmp((const char *)kkey, SVN_DAV_PROP_NS_SVN,
+ sizeof(SVN_DAV_PROP_NS_SVN) - 1) == 0)
+ entry->has_props = TRUE;
+ }
                 }
              }
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Feb 17 01:20:11 2006

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.