Re: svn commit: r1469982 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/log.c libsvn_client/ra.c tests/cmdline/log_tests.py

On Sat, Apr 20, 2013 at 5:00 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>> -----Original Message-----
>> From: pburba_at_apache.org [mailto:pburba_at_apache.org]
>> Sent: vrijdag 19 april 2013 20:22
>> To: commits_at_subversion.apache.org
>> Subject: svn commit: r1469982 - in /subversion/trunk/subversion:
>> include/private/svn_client_private.h libsvn_client/log.c libsvn_client/ra.c
>> tests/cmdline/log_tests.py
>>
>> Author: pburba
>> Date: Fri Apr 19 18:21:36 2013
>> New Revision: 1469982
>>
>> URL: http://svn.apache.org/r1469982
>> Log:
>> 2nd attempt to fix issue #4355 'svn_client_log5 broken with multiple
>> revisions which span a rename'.
>
> Hi,
>
> This patch and the few before extend svn_client_log to call more and more history operations on the same path.

Hi Bert,

The "few before"? There is only one (rr1469515) that I see. Are you
referring to work older than what I've done in the prior week? If so
could you point out the specific revs? I want to be sure I know what
you are referring to.

> (The path resolving via svn_client__repos_locations() is on the file system layer essentially the same thing as walking the history on the filesystem layer.)
>
> Instead of patching more and more corner cases with individual extra ra calls I think svn_client_log should call svn_client__repos_locations() once to obtain the history of the path to look at over the entire range of versions. (=over MAX(rev):MIN(rev)) and then use that information directly as the paths for the rest of the log calls.

How exactly would svn_client__repos_locations be useful? Do you mean
svn_ra_get_location_segments?

> libsvn_client's log operation is already a very slow operation over high latency links with mod_dav and this patching with extra ra requests is making it worse and worse.

I reverted r1469515 and r1469982 (excepting the new log tests of
course, I left those in), so we are back at square one.

> Performing this operation once should improve the log performance considerable, making the multi range version much better for script and api users.
>
> Bert
>

On Mon, Apr 22, 2013 at 9:46 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> On 04/20/2013 05:00 AM, Bert Huijben wrote:
>> Instead of patching more and more corner cases with individual extra ra
>> calls I think svn_client_log should call svn_client__repos_locations()
>> once to obtain the history of the path to look at over the entire range
>> of versions. (=over MAX(rev):MIN(rev)) and then use that information
>> directly as the paths for the rest of the log calls.
>
> Agreed. Was thinking exactly the same thing.
>
> I wonder, though: isn't this rev-range support better situated in the RA
> layer itself, extended up to the server? I ask because IIRC, the fallback
> implementation of svn_ra_get_repos_locations()

You too mean svn_ra_get_location_segments or svn_ra_get_locations I assume?

> uses none other than the 'log' functionality.

> So I see this:
>
> BEST CASE: client's RA layer and server can handle log-with-ranges
>
> FALLBACK 1: client RA layer works around server's lack of log-with-ranges
> support by using get_locations() and a series of log requests
>
> FALLBACK 2: client RA layer works around server's lack of log-with-ranges
> support and lack of get_locations support by using a single
> log request from which disinteresting revisions are merely filtered
> out.

--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
On Mon, Apr 22, 2013 at 10:21 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: C. Michael Pilato [mailto:cmpilato_at_collab.net]
>> Sent: maandag 22 april 2013 15:46
>> To: Bert Huijben
>> Cc: dev_at_subversion.apache.org
>> Subject: Re: svn commit: r1469982 - in /subversion/trunk/subversion:
>> include/private/svn_client_private.h libsvn_client/log.c libsvn_client/ra.c
>> tests/cmdline/log_tests.py
>>
>> On 04/20/2013 05:00 AM, Bert Huijben wrote:
>> > Instead of patching more and more corner cases with individual extra ra
>> > calls I think svn_client_log should call svn_client__repos_locations()
>> > once to obtain the history of the path to look at over the entire range
>> > of versions. (=over MAX(rev):MIN(rev)) and then use that information
>> > directly as the paths for the rest of the log calls.
>>
>> Agreed.  Was thinking exactly the same thing.
>>
>> I wonder, though:  isn't this rev-range support better situated in the RA
>> layer itself, extended up to the server?  I ask because IIRC, the fallback
>> implementation of svn_ra_get_repos_locations() uses none other than the
>> 'log' functionality.
>>
>> So I see this:
>>
>>   BEST CASE:  client's RA layer and server can handle log-with-ranges
>>
>>   FALLBACK 1:  client RA layer works around server's lack of log-with-ranges
>>       support by using get_locations() and a series of log requests
>
> I think this should work for 1.5+ servers.
>
>>   FALLBACK 2:  client RA layer works around server's lack of log-with-ranges
>>       support and lack of get_locations support by using a single
>>       log request from which disinteresting revisions are merely filtered
>>       out.
>
> +1
>
> Given that we already branched for 1.8 I would suggest backporting FALLBACK 1 to 1.8 and leaving the BEST CASE for 1.9.
>
> But maybe if somebody starts now, he/she can get it ready for 1.8.
> (Would be nice to see the performance improvements sooner)
>
> The fallback 1/2 cases can be implemented as a bugfix even for 1.7 if there is enough interest as they don't change public APIs.
>
>         Bert
>
-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba