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

New svn_path_is_root "X:" stuff makes Subversion sslllooowww

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2006-09-27 13:57:37 CEST

The new support for "X:" Windows paths has a substantial performance
penalty, on Unix at least. When running "svn st" on a pristine
working copy of the Subversion source code the new code needs about
four times the CPU and runtime of the old code. r21277 seems to be
the main culprit:

------------------------------------------------------------------------
r21277 | lgo | 2006-08-26 21:33:01 +0100 (Sat, 26 Aug 2006) | 35 lines

Prepare fix for issue #2556: abstract root folder checks in svn_path_is_root.
Add support for 'X:/' as a root folder on Windows.

Patch by: me
Review by: brane
           dionisos
           philip
(brane & dionisos basically helped me write the patch)
Approved by: dionisos

* subversion/include/svn_path.h
  (svn_path_is_root): New function declaration.
  (svn_path_is_empty): Added comment.

* subversion/libsvn_client/commit.c
  (svn_client_commit4): make sure to abort when the iterating through the
   parent folders passese the root folder.

* subversion/libsvn_subr/path.c
  (svn_path_is_root): New function. Tests for all forms considered canonical
   root specs on the platform at hand ('/', 'X:/' etc.)
  (is_canonical): 'X:/' syntax on Windows is canonical.
  (svn_path_join, svn_path_dirname, svn_path_basename,
   svn_path_join_many, previous_segment,
   get_path_ancestor_length): Support the new type of root path on Windows,
    mostly by replacing direct comparisons of path and '/' with a call to
    svn_path_is_root.
  (svn_path_canonicalize): don't strip the trailing slash if the path is
   of the 'X:/' syntax.

* subversion/tests/libsvn_subr/path-test.c
  (test_is_root): New test for validation of svn_path_is_root.
  (test_path_split, test_join,
   test_basename, test_remove_component): add extra cases for 'X:/'.
  (test_funcs): run the new test test_is_root.
------------------------------------------------------------------------

I can get most of the performance back by using this hack:

Index: subversion/libsvn_subr/path.c
===================================================================
--- subversion/libsvn_subr/path.c (revision 21676)
+++ subversion/libsvn_subr/path.c (working copy)
@@ -437,6 +437,7 @@
 svn_boolean_t
 svn_path_is_root(const char *path, apr_size_t len, apr_pool_t *pool)
 {
+#if 0
   const char *root_path = NULL;
   apr_status_t status;
   apr_pool_t *strpool = (pool) ? pool : svn_pool_create(NULL);
@@ -471,6 +472,9 @@
   if (!pool)
     apr_pool_destroy(strpool);
   return result;
+#else
+ return len == 1 && path[0] == '/';
+#endif
 }

but it's still not as fast as 1.4. I don't know whether the "X:"
stuff is responsible for the remaining performance regression. I'm
using a development build: unoptimised, pool debugging, etc., so
timings for a release build might vary, but I'd be surprised if it
made much difference.

I suppose we could make the expensive svn_path_is_root stuff Windows
specific, but really I don't think the current stuff is suitable
anywhere. When I added the is_canonical stuff it was intended to be
fast, I even commented out some of the calls I used during development
in order to avoid extra strlen calls. Now is_canonical is creating
pools and doing utf8 conversions, it's simply much too slow.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 27 13:57:56 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.