Re: SQLite and callbacks

On 05.02.2011 21:55, Johan Corveleyn wrote:
> On Sat, Feb 5, 2011 at 9:37 PM, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
>> On Sat, Feb 5, 2011 at 7:14 PM, Stefan Sperling <stsp_at_elego.de> wrote:
>>> On Sat, Feb 05, 2011 at 06:47:35PM +0100, Branko Čibej wrote:
>>>> I would not worry about existing clients -- simply mark the existing
>>>> APIs as deprecated, but keep them and do not attempt to improve their
>>>> performance.
>>> Neglecting performance of backwards compat code is an interesting idea.
>>> It allows us to focus on the new APIs first and foremost.
>>>
>>> The existing APIs will continue to work using the node walker and issue
>>> queries per node as they do now. We could consider optimising them later,
>>> before or after 1.7 release. The required changes are mostly mechanical.
>> I agree with most of what's been said here. I think it would be a pity
>> to use WC-NG in a way that provides far from optimal performance.
>>
>> FWIW, I just did a quick run of your per-directory proplist query vs.
>> the per-node version, on my Windows XP platform, to have another data
>> point. The performance improvement is significant, but not
>> earth-shattering (around 20%).
>>
>> Just tested with a freshly checked out working copy of svn trunk:
>>
>> 1) Per-node queries (r1066540). Looking at the third run, to make sure
>> everything is hot in cache:
>>
>> $ time svn proplist -R . >/dev/null
>>
>> real 0m1.532s
>> user 0m0.015s
>> sys 0m0.015s
>>
>>
>> 2) Per-dir queries (r1066541). Looking at the third run, to make sure
>> everything is hot in cache:
>>
>> $ time svn proplist -R . >/dev/null
>>
>> real 0m1.218s
>> user 0m0.015s
>> sys 0m0.031s
>>
>>
>> 3) For comparison, I also tested with SlikSVN 1.6.13. This is still
>> more than twice as fast:
>>
>> $ time svn proplist -R . >/dev/null
>>
>> real 0m0.578s
>> user 0m0.015s
>> sys 0m0.046s
> I should've added one more test, to put things in perspective :-),
> namely the test with r1039808 (the per-wc query version):
>
> $ time svn proplist -R . >/dev/null
>
> real 0m0.328s
> user 0m0.015s
> sys 0m0.031s
>
> (but, as you probably already know, this execution is not cancellable,
> which is pretty annoying :-), especially when I forget to redirect to
> /dev/null, but let it write on the console).
>
> Cheers,

Clearly the full-wc query is the best solution, and it's not really hard
to avoid the possible deadlocks associated with running callbacks from
within a transaction. And even the best current queries are doing way
too much in code instead of using the power of SQL to speed thing up.

As a proof of concept, I took the recursive proplist function, then
changed it it issue the callbacks outside sqlite transactions and made
the thing cancelable, by using a file-based temporary database to cache
the query results, and offloading most of the heavy lifting to sqlite.

Here's my resulton a fresh trunk working copy:

$ time svn proplist -R . >/dev/null

real 0m0.066s
user 0m0.047s
sys 0m0.014s

And here's current trunk without modifications, on the same working copy:
$ time svn proplist -R . >/dev/null

real 0m0.586s
user 0m0.549s
sys 0m0.029s

Just to put things in perspective, I ran both tests on a full checkout
of our tags directory:

With the patch:

$ time svn proplist -R . >/dev/null

real 0m4.926s
user 0m4.178s
sys 0m0.679s

Without the patch, I canceled the command after it had been running for
an hour, with CPU usage at more than 95%. So there's clearly quite a bit
of room for improving performance of the working copy.

Note that I didn't run the tests with this patch, so I'm not claiming it
to be bug-free. In fact there's one bug in the recursive proplist of the
WC root, where the root props show up twice -- there's an issue with the
filter in the query there. But it's proof-of-concept after all.

-- Brane

P.S.: By the way, checking out tags got the current trunk svn client
memory usage up to 2.5 gigs before it gave up and hung, and the WC was
borked, and I had to do the checkout in pieces, separately for each tag.
That's less than acceptable.