On Tue, Feb 15, 2011 at 12:44, Hyrum K Wright <hy...@hyrumwright.org> wrote: > On Tue, Feb 15, 2011 at 6:42 AM, Greg Stein <gst...@gmail.com> wrote: >> On Mon, Feb 14, 2011 at 14:14, C. Michael Pilato <cmpil...@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
