On Thu, 10 Nov 2005, Daniel Berlin wrote:
> This patch adds the ability to cache whether certain properties exist in
> the entries file. It does not cache their values.
>
There are stylistic stuff, but I see this as a prototype.
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 17287)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -1216,6 +1216,10 @@ typedef struct svn_wc_entry_t
> * @since New in 1.4. */
> svn_boolean_t prop_mods;
>
> + /** Cached property existence for this entry.
> + * @since New in 1.4. */
> + const char *has_properties;
> +
The problem with using a string for this is space. An apr_uint32_t
with bits for signle props would be more space-efficient than a little
string for each entry. NOt that we are terribly space-efficient for
the entry cache, but that's another topic.
> +/** Return a comma separated string containing the names of all of the
> + * properties that the working copy caches the existence of.
> + * @since New in 1.4.
> + */
> +const char *svn_wc_cached_properties (void);
> +
Why do we need this as a public API. I'd like to keep this as an
implementation detail.
>
> #ifdef __cplusplus
> }
> Index: subversion/libsvn_wc/props.c
> ===================================================================
> --- subversion/libsvn_wc/props.c (revision 17287)
> +++ subversion/libsvn_wc/props.c (working copy)
> @@ -314,9 +314,12 @@ svn_wc__install_props (svn_stringbuf_t *
> const char *full_path;
> const char *access_path = svn_wc_adm_access_path (adm_access);
> int access_len = strlen (access_path);
> + apr_array_header_t *cached_props;
> apr_array_header_t *prop_diffs;
> svn_wc_entry_t tmp_entry;
> svn_node_kind_t kind;
> + char *has_properties_string = NULL;
> + int i;
>
> /* Non-empty path without trailing slash need an extra slash removed */
> if (access_len != 0 && access_path[access_len - 1] != '/')
> @@ -342,11 +345,32 @@ svn_wc__install_props (svn_stringbuf_t *
> SVN_ERR (svn_prop_diffs (&prop_diffs, working_props, base_props, pool));
> tmp_entry.prop_mods = (prop_diffs->nelts > 0);
>
> +
> +
> + cached_props = svn_cstring_split (svn_wc_cached_properties (), ",",
> + TRUE, pool);
> + for (i = 0; i < cached_props->nelts; i++)
> + {
> + const char *proptolookfor = APR_ARRAY_IDX (cached_props, i, const char *);
> + if (apr_hash_get (working_props, proptolookfor, APR_HASH_KEY_STRING) != NULL)
> + {
> + if (!has_properties_string)
> + has_properties_string = "";
> + has_properties_string = apr_pstrcat (pool, has_properties_string,
> + proptolookfor, " ", NULL);
> + }
> + }
> +
> + tmp_entry.has_properties = has_properties_string;
> +
> /* ### TODO: Check props in working props and update flags for specific
> props when we have such flags. */
>
That TODO is for the above:-) But, sadly svn_wc__install_props is not
> the only place where we muck with the properties. There are more
> places to be updated, but let's discuss the general design
> first. So, this needs to be factored out in a separate function.
> Long-term we want to minimize the number of places that touches
> properties, as per Zhakov's proposed property abstraction API.
> @@ -1302,9 +1346,19 @@ svn_wc_prop_get (const svn_string_t **va
> {
> svn_error_t *err;
> apr_hash_t *prophash;
> -
> enum svn_prop_kind kind = svn_property_kind (NULL, name);
>
> + if (strstr (svn_wc_cached_properties (), name))
> + {
> + const svn_wc_entry_t *entry;
> + SVN_ERR (svn_wc_entry (&entry, path, adm_access, TRUE, pool));
> + if (!entry->has_properties || !strstr (entry->has_properties, name))
> + {
> + *value = NULL;
> + return SVN_NO_ERROR;
> + }
> + }
> +
We can be even smarter here for boolean props. Their existence implies
that they have the value "*", so we don't need to load the property
file at all in that case. (We could even avoid putting this in the
properties file at all in this case. Whether that complication is
worth it I don't know.)
> if (kind == svn_prop_wc_kind)
> {
> return svn_wc__wcprop_get (value, name, path, adm_access, pool);
> @@ -2147,3 +2201,9 @@ svn_wc__has_magic_property (apr_array_he
> }
> return FALSE;
> }
> +
> +const char *
> +svn_wc_cached_properties (void)
> +{
> + return SVN_PROP_SPECIAL "," SVN_PROP_EXTERNALS "," SVN_PROP_NEEDS_LOCK;
This list can be tweaked during the 1.4 cycle. Externals seem good. I
guess it speeds up update and status?
Thanks for this prototype, which confirms the improvements we hoped
for:-)
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Nov 10 21:45:37 2005