In various places in libsvn_client and libsvn_wc, we call
svn_wc_get_prop_diffs() to get the BASE properties for an entry.
In many cases (particularly during 'svn diff', but also in other places,
like 'svn proplist -rBASE'), we don't need the propchanges that that
function also returns, and so we pass in NULL for the propchanges
argument. However, svn_wc_get_prop_diffs() unconditionally reads in
the WORKING properties for each entry regardless of whether or not we've
asked for the diffs.
The attached patch changes svn_wc_get_prop_diffs() to avoid the redundant
load of the WORKING properties if propchanges aren't required. I've not
quantified the savings, and I doubt they're significant, but avoiding
reading unnecessary files is always a good idea, and the patch is
self-contained and passes 'make check'.
Two notes about the patch:
1. I thought that it was clearer to return early from the function if
propchanges was NULL, rather than wrap the second half of the function
in the body of an if() statement, but the disadvantage is that there
are now two exit points from the function. Let me know if you want it
the other way.
2. We could also move the initialisation of the localprops local
variable further down, but I assume that the call to apr_hash_make()
is inexpensive.
[[[
* subversion/libsvn_wc/props.c
(svn_wc_get_prop_diffs): Rearrange, and exit early if the caller doesn't
want to receive property changes, avoiding an unnecessary read of
the WORKING properties file.
]]]
Regards,
Malcolm
Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c (revision 17144)
+++ subversion/libsvn_wc/props.c (working copy)
@@ -1807,26 +1807,28 @@ svn_wc_get_prop_diffs (apr_array_header_
apr_hash_t *localprops = apr_hash_make (pool);
apr_hash_t *baseprops = apr_hash_make (pool);
-
- SVN_ERR (svn_wc__prop_path (&prop_path, path, adm_access, FALSE, pool));
+ /* Load the BASE props. */
SVN_ERR (svn_wc__prop_base_path (&prop_base_path, path, adm_access, FALSE,
pool));
-
- SVN_ERR (svn_wc__load_prop_file (prop_path, localprops, pool));
SVN_ERR (svn_wc__load_prop_file (prop_base_path, baseprops, pool));
if (original_props != NULL)
*original_props = baseprops;
+ /* If we don't care about the propchanges, we can quit now. */
+ if (propchanges == NULL)
+ return SVN_NO_ERROR;
+
+ /* Load the WORKING props. */
+ SVN_ERR (svn_wc__prop_path (&prop_path, path, adm_access, FALSE, pool));
+ SVN_ERR (svn_wc__load_prop_file (prop_path, localprops, pool));
+
/* At this point, if either of the propfiles are non-existent, then
the corresponding hash is simply empty. */
-
- if (propchanges != NULL)
- {
- SVN_ERR (svn_prop_diffs (&local_propchanges, localprops,
- baseprops, pool));
- *propchanges = local_propchanges;
- }
+
+ SVN_ERR (svn_prop_diffs (&local_propchanges, localprops,
+ baseprops, pool));
+ *propchanges = local_propchanges;
return SVN_NO_ERROR;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 2 17:55:31 2005