You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/04/17 19:40:58 UTC
Re: svn commit: r30666 - trunk/subversion/libsvn_ra_neon
kfogel@tigris.org writes:
> Log:
> In ra_neon, cache starting props when first seen, to reduce round trips.
>
> Patch by: Daniel Shahaf <d....@daniel.shahaf.co.il>
> (Tweaked by me.)
Daniel, I promised I'd tell you what the tweaks were. Here they are:
This documenting of the new behaviors is something I added.
> --- trunk/subversion/libsvn_ra_neon/props.c (r30665)
> +++ trunk/subversion/libsvn_ra_neon/props.c (r30666)
> @@ -638,8 +638,32 @@ svn_error_t * svn_ra_neon__get_starting_
> const char *label,
> apr_pool_t *pool)
> {
> - return svn_ra_neon__get_props_resource(rsrc, sess, url, label, starting_props,
> - pool);
> + svn_string_t *propval;
> +
> + SVN_ERR(svn_ra_neon__get_props_resource(rsrc, sess, url, label,
> + starting_props, pool));
> +
> + /* Cache some of the resource information. */
> +
> + if (! sess->vcc)
> + {
> + propval = apr_hash_get((*rsrc)->propset,
> + SVN_RA_NEON__PROP_VCC,
> + APR_HASH_KEY_STRING);
> + if (propval)
> + sess->vcc = apr_pstrdup(sess->pool, propval->data);
> + }
> +
> + if (! sess->uuid)
> + {
> + propval = apr_hash_get((*rsrc)->propset,
> + SVN_RA_NEON__PROP_REPOSITORY_UUID,
> + APR_HASH_KEY_STRING);
> + if (propval)
> + sess->uuid = apr_pstrdup(sess->pool, propval->data);
> + }
> +
> + return SVN_NO_ERROR;
> }
As you can see, I rearranged the code a bit here, putting conditional
checks earlier, using braces, etc.
> + if (! sess->vcc)
> + {
> + /* ### better error reporting... */
> + return svn_error_create(APR_EGENERAL, NULL,
> + _("The VCC property was not found on the "
> + "resource"));
> + }
Added braces.
> --- trunk/subversion/libsvn_ra_neon/ra_neon.h (r30665)
> +++ trunk/subversion/libsvn_ra_neon/ra_neon.h (r30666)
> @@ -98,6 +98,10 @@ typedef struct {
> svn_auth_iterstate_t *p11pin_iterstate; /* state of PKCS#11 pin retries */
>
> svn_boolean_t compression; /* should we use http compression? */
> +
> + /* Both of these function as caches, and are NULL when uninitialized
> + or cleared: */
> + const char *vcc; /* version-controlled-configuration */
> const char *uuid; /* repository UUID */
>
> svn_ra_progress_notify_func_t progress_func;
Added the comment about how they're caches.
> @@ -423,7 +427,10 @@ svn_error_t * svn_ra_neon__get_props_res
> const ne_propname *which_props,
> apr_pool_t *pool);
>
> -/* fetch a single resource's starting props from the server. */
> +/* fetch a single resource's starting props from the server.
> +
> + Cache the version-controlled-configuration in SESS->vcc, and the
> + repository uuid in SESS->uuid. */
> svn_error_t * svn_ra_neon__get_starting_props(svn_ra_neon__resource_t **rsrc,
> svn_ra_neon__session_t *sess,
> const char *url,
> @@ -437,7 +444,10 @@ svn_error_t * svn_ra_neon__get_starting_
>
> Also return *MISSING_PATH (allocated in POOL), which is the
> trailing portion of the URL that did not exist. If an error
> - occurs, *MISSING_PATH isn't changed. */
> + occurs, *MISSING_PATH isn't changed.
> +
> + Cache the version-controlled-configuration in SESS->vcc, and the
> + repository uuid in SESS->uuid. */
> svn_error_t *
> svn_ra_neon__search_for_starting_props(svn_ra_neon__resource_t **rsrc,
> const char **missing_path,
Added documentation about the caching behavior.
> @@ -423,7 +427,10 @@ svn_error_t * svn_ra_neon__get_props_res
> const ne_propname *which_props,
> apr_pool_t *pool);
>
> -/* fetch a single resource's starting props from the server. */
> +/* fetch a single resource's starting props from the server.
> +
> + Cache the version-controlled-configuration in SESS->vcc, and the
> + repository uuid in SESS->uuid. */
> svn_error_t * svn_ra_neon__get_starting_props(svn_ra_neon__resource_t **rsrc,
> svn_ra_neon__session_t *sess,
> const char *url,
> @@ -437,7 +444,10 @@ svn_error_t * svn_ra_neon__get_starting_
>
> Also return *MISSING_PATH (allocated in POOL), which is the
> trailing portion of the URL that did not exist. If an error
> - occurs, *MISSING_PATH isn't changed. */
> + occurs, *MISSING_PATH isn't changed.
> +
> + Cache the version-controlled-configuration in SESS->vcc, and the
> + repository uuid in SESS->uuid. */
> svn_error_t *
> svn_ra_neon__search_for_starting_props(svn_ra_neon__resource_t **rsrc,
> const char **missing_path,
>
> Modified: trunk/subversion/libsvn_ra_neon/session.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_neon/session.c?pathrev=30666&r1=30665&r2=30666
> ==============================================================================
> --- trunk/subversion/libsvn_ra_neon/session.c (r30665)
> +++ trunk/subversion/libsvn_ra_neon/session.c (r30666)
> + if (! ras->uuid)
> + {
> + /* ### better error reporting... */
> + return svn_error_create(APR_EGENERAL, NULL,
> + _("The UUID property was not found on the "
> + "resource or any of its parents"));
> + }
> }
Added braces.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r30666 - trunk/subversion/libsvn_ra_neon
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Karl Fogel wrote on Thu, 17 Apr 2008 at 15:40 -0400:
> kfogel@tigris.org writes:
> > Log:
> > In ra_neon, cache starting props when first seen, to reduce round trips.
> >
> > Patch by: Daniel Shahaf <d....@daniel.shahaf.co.il>
> > (Tweaked by me.)
>
> Daniel, I promised I'd tell you what the tweaks were. Here they are:
>
>
> > --- trunk/subversion/libsvn_ra_neon/props.c (r30665)
> > +++ trunk/subversion/libsvn_ra_neon/props.c (r30666)
> > @@ -638,8 +638,32 @@ svn_error_t * svn_ra_neon__get_starting_
> > + if (! sess->uuid)
> > + {
> > + propval = apr_hash_get((*rsrc)->propset,
> > + SVN_RA_NEON__PROP_REPOSITORY_UUID,
> > + APR_HASH_KEY_STRING);
> > + if (propval)
> > + sess->uuid = apr_pstrdup(sess->pool, propval->data);
> > + }
> > +
> > + return SVN_NO_ERROR;
> > }
>
> As you can see, I rearranged the code a bit here, putting conditional
> checks earlier, using braces, etc.
>
OK, I missed that.
> > + if (! sess->vcc)
> > + {
> > + /* ### better error reporting... */
> > + return svn_error_create(APR_EGENERAL, NULL,
> > + _("The VCC property was not found on the "
> > + "resource"));
> > + }
>
> Added braces.
>
OK.
> > --- trunk/subversion/libsvn_ra_neon/ra_neon.h (r30665)
> > +++ trunk/subversion/libsvn_ra_neon/ra_neon.h (r30666)
> > @@ -98,6 +98,10 @@ typedef struct {
> > svn_auth_iterstate_t *p11pin_iterstate; /* state of PKCS#11 pin retries */
> >
> > svn_boolean_t compression; /* should we use http compression? */
> > +
> > + /* Both of these function as caches, and are NULL when uninitialized
> > + or cleared: */
> > + const char *vcc; /* version-controlled-configuration */
> > const char *uuid; /* repository UUID */
> >
> > svn_ra_progress_notify_func_t progress_func;
>
> Added the comment about how they're caches.
>
OK. The comments I put were not very helpful, indeed.
> > @@ -437,7 +444,10 @@ svn_error_t * svn_ra_neon__get_starting_
> >
> > Also return *MISSING_PATH (allocated in POOL), which is the
> > trailing portion of the URL that did not exist. If an error
> > - occurs, *MISSING_PATH isn't changed. */
> > + occurs, *MISSING_PATH isn't changed.
> > +
> > + Cache the version-controlled-configuration in SESS->vcc, and the
> > + repository uuid in SESS->uuid. */
> > svn_error_t *
> > svn_ra_neon__search_for_starting_props(svn_ra_neon__resource_t **rsrc,
> > const char **missing_path,
>
> Added documentation about the caching behavior.
>
OK.
> > --- trunk/subversion/libsvn_ra_neon/session.c (r30665)
> > +++ trunk/subversion/libsvn_ra_neon/session.c (r30666)
> > + if (! ras->uuid)
> > + {
> > + /* ### better error reporting... */
> > + return svn_error_create(APR_EGENERAL, NULL,
> > + _("The UUID property was not found on the "
> > + "resource or any of its parents"));
> > + }
> > }
>
> Added braces.
>
> -Karl
>
Thanks for the review Karl!
--
Daniel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r30666 - trunk/subversion/libsvn_ra_neon
Posted by Karl Fogel <kf...@red-bean.com>.
Karl Fogel <kf...@red-bean.com> writes:
> This documenting of the new behaviors is something I added.
Oops, I said that twice, because I made an editing mistake in my email.
Sorry for the confusion.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org