[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: Philip Martin <philip_at_codematters.co.uk>
Date: 2004-10-23 15:35:21 CEST

Martin Hauner <hauner@web.de> writes:

> * subversion/libsvn_wc/props.c
> (svn_wc__load_prop_file): removed an unecessary svn_io_check_path
> call. svn_wc__load_prop_file already does the check.

That looks good; I'll look at committing it.

> Removed svn_io_check_path calls to check for links which are uncessary
> on the Win32 plattform.
>
> * subversion/libsvn_wc/status.c
> (assemble_status): Initializing wc_special and node_special to false.
> Wrapped 'special' checks in #ifndef _WIN32.
>
> Index: subversion/libsvn_wc/status.c
> ===================================================================
> --- subversion/libsvn_wc/status.c (revision 11539)
> +++ subversion/libsvn_wc/status.c (working copy)
> @@ -212,8 +212,8 @@
> svn_boolean_t prop_modified_p = FALSE;
> svn_boolean_t locked_p = FALSE;
> svn_boolean_t switched_p = FALSE;
> - svn_boolean_t wc_special;
> - svn_boolean_t node_special;
> + svn_boolean_t wc_special = FALSE;
> + svn_boolean_t node_special = FALSE;
> svn_node_kind_t kind;
>
> /* Defaults for two main variables. */
> @@ -223,7 +223,6 @@
> /* Check the path kind for PATH. */
> if (path_kind == svn_node_unknown)
> SVN_ERR (svn_io_check_path (path, &path_kind, pool));
> - SVN_ERR (svn_io_check_special_path (path, &kind, &node_special, pool));
>
> if (! entry)
> {
> @@ -305,7 +304,12 @@
> SVN_ERR (svn_wc_props_modified_p (&prop_modified_p, path, adm_access,
> pool));
>
> +#ifndef _WIN32
> + // we don't have links on win32, so we can save some io_check_path calls
> + // on the file itself and its property file.
> + SVN_ERR (svn_io_check_special_path (path, &kind, &node_special, pool));
> SVN_ERR (svn_wc__get_special (&wc_special, path, adm_access, pool));
> +#endif // _WIN32

The rest of the code use WIN32, not _WIN32. I don't do Windows so I
can't really comment, but I don't like the spread of #ifdef code. I
haven't looked in detail, but my initial instinct is that the
conditional code should be inside the functions, not at the call site.

> * subversion/libsvn_wc/questions.c
> (svn_wc_text_modified_p2): copied from svn_wc_text_modified_p and
> removed the svn_io_check_path check.
> (svn_wc_text_modified_p): Does the svn_io_check_path check and then
> calls svn_wc_text_modified_p2.
>
> * subversion/libsvn_wc/status.c
> (assemble_status): call svn_wc_text_modified_p2 instead of
> svn_wc_text_modified_p.

Our naming usually uses foo2 when foo2 is intended to replace foo.
Since you don't appear to be replacing all the calls that's not really
the case here, is it?

The text_modified_p and props_modified_p functions have long been on
my hit list, they are really inefficient in terms of disk io and iconv
stuff (the props_modified_p function is particularly bad). I'd prefer
to see the modified_p functions implemented so that they don't
repeatedly stat(), then all callers would benefit.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 23 15:36:21 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.