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

Re: [wc-propcaching]: Cache existence of certain properties

From: Daniel Berlin <dberlin_at_dberlin.org>
Date: 2005-11-10 21:59:02 CET

>
> > 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.

This is true, and if you want, i'll move it to a uint32.
But i think we should still write it out as a string.

>
> > +/** 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.

Okay, that's fine.
So do you want this to be an int, or a string?

>
> >
> > #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.

If you stare about, you will discover this really is the only place that
seems to need to be changed.
The other places you might think need to changed (like prop_set2) call
install_props.

So where else are you thinking we need to change?

> 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.)

True. I didn't bother with this yet.

>
> > 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.

Every time you add to the list, you will break existing wc's with newer
clients. Every time you delete from the list, you will break newer wc's
with older clients. :)

> Externals seem good. I
> guess it speeds up update and status?

Yes.

---------------------------------------------------------------------
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:59:53 2005

This is an archived mail posted to the Subversion Dev mailing list.