Re: [PATCH] Speed up client by re-using RA session connections

On Thu, Sep 15, 2011 at 01:13, Hyrum K Wright <hyrum.wright_at_wandisco.com> wrote:
> On Wed, Sep 14, 2011 at 4:00 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>> On Wed, Sep 14, 2011 at 19:07, Julian Foad <julian.foad_at_wandisco.com> wrote:
>>> A patch in progress, for demonstration and your feedback.
>>>
>>> Since long ago we've thought about letting the client, through the
>>> libsvn_client API, share a single RA connection across a series of
>>> operations -- e.g.
>>>
>>>  "svn update a b c" currently opens 3 RA sesssions.
>>>
>>> Similarly, inside libsvn_client we often open up an extra RA session
>>> when we could have re-used an old one -- e.g.
>>>
>>>  Update with externals -- opens a new session per external [1]
>>>
>>>  "svn mergeinfo" -- opens two or three sessions
>>>
>>> The attached patch implements "caching" of connections that have been
>>> used and may be used again.  The cache is initialized (to empty) by the
>>> caller (the client executable) and connections in it are established or
>>> re-used by libsvn_client as required.
>>>
>> Hi Julian,
>>
>> My original idea was to make ra session pool (cache) to be part of
>> svn_client_context_t, so callers of svn_client_* API do not have to be
>> modified. And pattern usage of this pool would by something like:
>>
>> svn_ra_session_t * ra = svn_client__ra_pool_get_session(ctx, pool)
>> [perform some operations with RA session]
>>
>> then call
>>
>> svn_client_ra_pool_release_session(ctx, ra);
>>
>> (RA session will be automatically released back to session pool
>> (cache) when pool used in svn_client__ra_pool_get_session() is
>> cleared)
>>
>> What do you think?
>
> I'll note that we may not even need an explicit release function, as
> the current ra session infrastructure simply installs a pool cleanup
> hook to do proper shutdown.  We could modify this hook to release the
> ra session back to the cache, rather than cleaning up the session (and
> in turn, the pool in which the cache is allocated could cleanup the
> cache and its contents).
>
> In all this we need to remember long-lived clients, and be sure we
> give them a way to kill the cache when it suites them.
>
It still worth to have explicit release function for cached RA
sessions, like we have for files and streams. Even they're
automatically closed on pool cleanup we often need just to say 'please
close this file, I'm finished working with it".

-- 
Ivan Zhakov