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

Re: [PATCH]? svn status performance (win32)

From: Branko Čibej <brane_at_xbc.nu>
Date: 2004-10-23 16:41:36 CEST

Martin Hauner wrote:

> Hi,
>
> at work we have a working copy that is not really small. It takes
> about 25 seconds to run svn status on it (from subcommander, build
> against the 1.1.x branch with svn_utf_initialize()).
>
> To find out what takes the time i did run it from Quantify.
>
> Subversion spends a lot of time in io_check_path (-> apr_stat ->
> GetFileAttributesEx).
>
> I added a printf(filename) to io_check_path to find out if it is
> possible to reduce the number of calls.
> For each file in the wc io_check_path is called three times on the
> file itself and two times on its property file.
>
> I created three simple patches that reduce the 5 io_check_path calls
> to a single call. They reduce the time needed for a status run about
> ~40% (not quantified,
> i am at home). What do you think?
>
> props.patch: removes an unecessary call on the properties file in
> svn_wc__load_prop_file. svn_wc__load_prop_file calls
> svn_wc__load_prop_file which does the same check.

This patch looks perfectly O.K. to me.
Well, the last SVN_ERR is redundant as you could just return the value
you got from svn_wc__load_prop_file. Committed with this change in
11592. Thanks!

Actually I suspect this patch gives most of the speedup. I get about 60%
shorter times on a "svn st -v" of a SVN trunk working copy, compared to
the 1.1.0 release. It also speeds up my ra_local tests by about 30%,
which is quite a feat, given that I'm running them on a ramdisk. All in
all, a good catch.

(Of course, your log message is wrong, but I took care of that. :-)

> status.patch: There are two calls to io_check_path here which are used
> to check for a 'special' file. As far as I see the only special case
> is a link. As links are not used on Win32 i #ifndef'ed them.

This one I don't like on philosophical grounds. If we do make
special-file checks platform-specific -- something I don't like, either
-- then the ifdef belongs inside svn_io_check_special_path, not in its
callers.

> statusquestion.patch: Here I removed the io_check_path call in
> svn_wc_text_modified_p. I created an svn_wc_text_modified_p2 method
> which does no io_check_path call. The original svn_wc_text_modified_p
> does the check and then calls svn_wc_text_modified_p2.

You didn't actually gain anything by doing this, did you? All you did
was move the check from one function to another. Also creating a new
public API is /not/ a good idea here (not to mention that you didn't
declare it in any header)

My guess is that you meant to make the ..._p2 function private in
libsvn_wc and have those functions that already do the check and then
call svn_wc_text_modified_p call the private function instead.

> All calls to svn_wc_text_modified_p do in some way the io_check_path
> check. So maybe just removing the check here is an option?

No, because svn_wc_text_modified_p is a public API, and you can't change
its behaviour.

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 23 16:42:11 2004

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.