Re: [PATCH] defer expensive load_ref_decorations until needed

From: Phil Hord <phil.hord@gmail.com>
Date: Nov 22, 2017, 23:21 UTC
Message-ID: <CABURp0rq9pwFWuBbrSB-FNUQ6B-7V8uL=Drw6O1-151u_cRKww@mail.gmail.com>
In-Reply-To: <20171122212710.GB2854@sigill>

On Wed, Nov 22, 2017 at 1:27 PM, Jeff King <peff@peff.net> wrote:

Show 14 quoted lines

> On Tue, Nov 21, 2017 at 03:43:36PM -0800, Phil Hord wrote:
>
>> With many thousands of references, a simple `git rev-parse HEAD` may take
>> more than a second to return because it first loads all the refs into
>> memory even though it will never use them.
>
> The overall goal of lazy-loading seems reasonable, but I'm slightly
> confused: how and why does "git rev-parse HEAD" load ref decorations?
>
> Grepping around I find that we mostly load them only when appropriate
> (when the "log" family sees a decorate option, when we see %d/%D in a
> pretty format, or with --simplify-by-decoration in a traversal). And
> poking at "rev-parse HEAD" in gdb seems to confirm that it does not hit
> that function.

Hm. I think I was confused.

I wrote v1 of this patch a few months ago. Clearly I was wrong about rev-parse being afflicted. We have a script that was suffering and it uses both "git log --format=%h" and "git rev-parse" to get hashes; I remember testing both, but I can't find it in my $zsh_history; my memory and my commit-message must be faulty.

However, "git log" does not need any --decorate option to trigger this lag.

    $ git for-each-ref| wc -l
    24172
    $ time git log --format=%h -1
    git log --format=%h -1   0.47s user 0.04s system 99% cpu 0.509 total

I grepped the code just now, too, and I see the same as you, though; it seems to hold off unless !!decoration_style. Nevertheless, gdb shows me decoration_style=1 with this command:

    GIT_CONFIG=/dev/null cgdb --args git log -1 --format="%h"

Here are timing tests on this repo without this change:

    git log --format=%h -1             0.54s user 0.05s system 99% cpu
0.597 total
    git log --format=%h -1 --decorate  0.54s user 0.04s system 98% cpu
0.590 total
    git log --format=%h%d -1           0.53s user 0.05s system 99% cpu
0.578 total

And the same commands with this change:

    git log --format=%h -1              0.01s user 0.01s system 71%
cpu 0.017 total
    git log --format=%h -1 --decorate   0.00s user 0.01s system 92%
cpu 0.009 total
    git log --format=%h%d -1            0.53s user 0.09s system 88%
cpu 0.699 total

> I have definitely seen "rev-parse HEAD" be O(# of refs), but that is
> mostly attributable to having all the refs packed (and until v2.15.0,
> the packed-refs code would read the whole file into memory).

Hm. Could this be why rev-parse was slow for me? My original problem showed up on v1.9 (build machine) and I patched it on v2.14.0-rc1. But, no; testing on 1.9, 2.11 and 2.14 still doesn't show me the lag in rev-parse. I remain befuddled.

> I've also
> seen unnecessary ref lookups due to replace refs (we load al of the
> packed refs to find out that no, there's nothing in refs/replace).

I haven't seen this in the code, but I have had refs/replace hacks in the past. Is that enough to wake this up?

Phil