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

Re: svn commit: r1657797 - /subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c

From: Branko Čibej <brane_at_wandisco.com>
Date: Fri, 06 Feb 2015 14:50:07 +0100

On 06.02.2015 14:13, ivan_at_apache.org wrote:
> Author: ivan
> Date: Fri Feb 6 13:13:14 2015
> New Revision: 1657797
>
> URL: http://svn.apache.org/r1657797
> Log:
> On the reuse-ra-session branch: Avoid svn_ra_reparent() call and network
> round-trip for svn:// protocol if possible.
>
> * subversion/libsvn_client/ra_cache.c
> (find_session_by_url): Try to find RA session with exact session URL
> match first.
>
> Modified:
> subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c
>
> Modified: subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c
> URL: http://svn.apache.org/viewvc/subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c?rev=1657797&r1=1657796&r2=1657797&view=diff
> ==============================================================================
> --- subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c (original)
> +++ subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c Fri Feb 6 13:13:14 2015
> @@ -401,6 +401,24 @@ find_session_by_url(cache_entry_t **cach
> {
> cache_entry_t *cache_entry;
>
> + /* Try to find RA session with exact session URL match first because
> + * the svn_ra_reparent() for svn:// protocol requires network round-trip.
> + */
> + APR_RING_FOREACH(cache_entry, &ra_cache->freelist,
> + cache_entry_t, freelist)
> + {
> + const char *session_url;
> + SVN_ERR_ASSERT(cache_entry->owner_pool == NULL);
> +
> + SVN_ERR(svn_ra_get_session_url(cache_entry->session, &session_url,
> + scratch_pool));
> + if (strcmp(session_url, url) == 0)
> + {
> + *cache_entry_p = cache_entry;
> + return SVN_NO_ERROR;
> + }
> + }

I think this is a mistake. We'll have to do a round-trip anyway to
ensure that the session is in a valid state (no connection expired,
etc.) before we can safely reuse it. The point of the cache is not to
avoid nework round-trips but to avoid expensive connection establishment.

I've kept this in for now when I added expiry handling, but I think that
in the long run, this change is trying to optimize for the wrong case.

-- Brane
Received on 2015-02-06 14:52:56 CET

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