[Monotone-devel] Re: [PATCH 1/2] git_export: avoid multiple sql queries

[Felipe Contreras]

On Sun, Mar 8, 2009 at 10:40 PM, Derek Scherger <address@hidden> wrote:
>
> On Sun, Mar 8, 2009 at 1:01 PM, Felipe Contreras
> <address@hidden> wrote:
>>
>> Yeah, I think there must be something fishy in my system, that's why
>> you were getting better performance all the time, but hopefully with
>> this patch my system (and other ones) will perform better.
>>
>> I'm interested in the Pidgin repo, I've been using my mtn2git script
>
> For reference, exporting a pidgin repo I downloaded a while ago takes about
> 70 minutes now and I think your patch should cut that down to about 40
> minutes but I haven't tested it yet. I'll let you know when I do.

It was taking two hours here, now it takes 60 min. There was quite a
big difference between your system and mine but my patch seems to
decrease the difference significantly.

So whatever it was, it's less relevant now.

>> that is also doing only a few sql queries but unfortunately that
>> generates different results than git_export. So I made the changes to
>> do the queries exactly as you do, and *bang*... slow as hell. My bet
>> is my sqlite (3.5.9).
>
> I seem to have sqlite 3.6.6.2 and the same problem. My guess is that it's
> not using an index that it should be but so far that's just a guess and I
> need to dig into it further. Not sure when I'll get to that. I did try a
> similar change to mtn log which should be suffering from the same problem
> but it didn't make much difference there.
>
>> I still need to push some stuff, like an option to find out missing
>> authors from the map.
>>
>> After finding this I decided to profile the 'loading' step, since IMO
>> it's taking too much time. I used gprof and gprof2dot and it looks
>>
>> like the biggest offender is get_change->roster_t::get_name taking 60%
>> of the time. That is after my modifications, which I guess can't be
>> applied upstream but maybe you would like to take a look?
>
> Sure, it can't hurt to look. If getting names from rosters is slow and we
> can speed it up then great.

Ok, attached. These are just for testing purposes I'm not even sure
how much each patch improves performance if any at all.

All I know is last time I measured the time it takes to import the
Pidgin repo it was 44min (from 60 in current mainline). It was
different baseline and different CFLAGS... it might not be due to the
patches.

>> Cool, it would be interesting to find out what caused this.
>
> If I fiind anything I'll let you know.
>
>>
>> I don't know... it's a normal patch with some extra info. It looks
>> like the patches came from mtn revision
>> '44683b999fa8092a1e7111728cf72e429b0abd0d'.
>
> It wasn't that it failed to apply cleanly, it was that patch didn't like it
> at all:
>
> $ patch -p0 < export1.patch
> patching file git_export.cc
> Hunk #1 FAILED at 215.
> patch: **** malformed patch at line 13:       cert_vector tags;

It could be the way you saved it from your mailer.

I just tried
patch -p1 -i 0001-Add-get_roster_version_fast.patch

No problem at all.

Now I'm sending the patches as attachments, maybe that helps.

Cheers.

-- 
Felipe Contreras

0001-Add-get_roster_version_fast.patch
Description: Binary data

0002-Add-get_name_fast.patch
Description: Binary data

0003-Random-cleanups.patch
Description: Binary data