On Tue, Feb 15, 2011 at 12:44, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Tue, Feb 15, 2011 at 6:42 AM, Greg Stein <gstein_at_gmail.com> wrote:
>> On Mon, Feb 14, 2011 at 14:14, C. Michael Pilato <cmpilato_at_collab.net> wrote:
>>> On 02/14/2011 01:57 PM, Ivan Zhakov wrote:
>>>> Hi,
>>>>
>>>> Currently ra_serf caches *all* DAV properties retrieved using PROPFIND
>>>> in session pool. This was attempt to reduce number of PROPFIND
>>>> requests. But current implementation has several problems:
>>>> 1. Unlimited memory usage: current implementation stores all
>>>> properties, so svn ls -R command can easily eat 150 mb of memory.
>>>> 2. Very low cache hit due the fact that cache doesn't support caching
>>>> "allprop" requests. For some operations there is no hits at all.
>>>> 3. Current implementation caches properties that may change between
>>>> requests, like URL to youngest revision.
>>>>
>>>> Of course these problems can be fixed, but I'm not sure that we need
>>>> this code. Since in svn 1.7 we have HTTPv2 which doesn't use so many
>>>> PROPFIND requests that we tried to reduce using DAV properties cache.
>>>>
>>>> So I'm propose just to remove DAV properties cache from ra_serf.
>>>> Objections? Comments?
>>>>
>>>> PS: See attached patch in case you'd like test performance.
>>>
>>> Ivan,
>>>
>>> Thanks so much for looking so closely into ra_serf recently! It hadn't
>>> occurred to me when implementing the HTTPv2 stuff that the prop cache might
>>> become less useful as a result.
>>>
>>> But what is the effect for older servers? Would it make sense keep the
>>> cache logic, but conditionally use it only for non-HTTPv2 connections?
>>
>> Note that removing the cache will not *break* the code. It simply
>> makes operation a bit slower.
>>
>> "upgrade your server"
>>
>> Given that we can simplify the client (remove caching), and given that
>> we can avoid unbounded memory usage, and that we don't really need the
>> cache... it seems like a win.
>
> How far can we go with this? If we're not using cached DAV properties
> with ra_serf, and ra_serf is the default in 1.7, does it make sense to
> remove support for them from the 1.7 working copy? It shouldn't be
> too difficult, and would tighten libsvn_wc up a bit.
>
You're talking about different thing: 'wcprops', DAV properties stored
in working copy. Actually ra_serf with HTTPv2 doesn't use them at all.
Even usage with server without HTTPv2 support is very limited. Also
I'm going to implement baseline information caching to ra_serf which
eliminate several PROPFINDs and make the situation better when server
doesn't support HTTPv2.
So my personal opinion that it's good idea to completely remove
'wcprops' support from libsvn_wc in 1.7 assuming that will simplify
the code and improve WCNG performance. As Greg said: "upgrade your
server".
--
Ivan Zhakov
Received on 2011-02-17 21:28:12 CET