The overhead of bin-wrappers/ (was: [PATCH 8/8] dir: avoid removing the current working directory)

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 26, 2021, 21:40 UTC
Message-ID: <211126.868rxaioem.gmgdl@evledraar.gmail.com>
In-Reply-To: <211125.86y25ciisr.gmgdl@evledraar.gmail.com>

On Thu, Nov 25 2021, Ævar Arnfjörð Bjarmason wrote:

Show 45 quoted lines

> On Wed, Nov 24 2021, Elijah Newren wrote:
>
>> On Wed, Nov 24, 2021 at 6:18 AM Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
> [...]
>>> I really wish we didn't have the bin-wrappers...
>>
>> I'm curious: is this issue the only reason, or am I reading between
>> the lines correctly that you have additional reasons for that?
>
> I think it sucks in general :)
>
> This is a much bigger change than what's needed for this topic or the
> proposed setup.c change I've got, but what we should be doing in general
> with bin-wrapper is to not have them at all under RUNTIME_PREFIX=Y
> (which we can make the default), and simply have a built/ directory or
> something, the structure of that directory will then 1=1 map to all the
> relative paths you'd get if you did a "make install".
>
> Currently in a built checkout you can do:
>
>     ./git version
>
> But not:
>
>     # need to add --exec-path=$PWD
>     ./git clone <http url>
>
> Or:
>
>     ./git send-email ...
>
> Which all comes down to sillyness in how we try to discover these paths,
> the *.mo files etc., there's also Perl-specific caveats in there.
>
> We'd still need something like a shellscript wrapper for the case of
> GIT_DEBUGGER, but we could just move that to being a compile-time flag,
> i.e. we'd swap out the "git" binary for a shellscript that chain-loaded
> to the "real" binary.
>
> It probably matters a bit for e.g. test suite performance, currently we
> spawn an extra shell for every single "git" invocation, but I mostly
> care about it for correctness, e.g. in this case of $PWD munging, and
> being able to have running a built binary from a checkout Just Work
> without the wrapper.

It also adds quite a bit of overhead to the tests, more than I vaguely
recalled. It differs by test, but here's one that invokes "git" a
lot". Trying with GIT_TEST_INSTALLED is a handy way to avoid the
bin-wrappers:
    
    $ hyperfine -L e ,GIT_TEST_INSTALLED=/home/avar/g/git/installed/bin '{e} ./t3070-wildmatch.sh --root=/dev/shm/git'
    Benchmark 1:  ./t3070-wildmatch.sh --root=/dev/shm/git
      Time (mean ± σ):      4.385 s ±  0.028 s    [User: 3.730 s, System: 0.824 s]
      Range (min … max):    4.340 s …  4.430 s    10 runs
     
    Benchmark 2: GIT_TEST_INSTALLED=/home/avar/g/git/installed/bin ./t3070-wildmatch.sh --root=/dev/shm/git
      Time (mean ± σ):      3.737 s ±  0.036 s    [User: 3.240 s, System: 0.656 s]
      Range (min … max):    3.686 s …  3.796 s    10 runs
     
    Summary
      'GIT_TEST_INSTALLED=/home/avar/g/git/installed/bin ./t3070-wildmatch.sh --root=/dev/shm/git' ran
        1.17 ± 0.01 times faster than ' ./t3070-wildmatch.sh --root=/dev/shm/git'

So a ~15-20% speedup, a more common case of a ~10% speedup:
    
    $ hyperfine -L e ,GIT_TEST_INSTALLED=/home/avar/g/git/installed/bin '{e} ./t0001-init.sh --root=/dev/shm/git'
    Benchmark 1:  ./t0001-init.sh --root=/dev/shm/git
      Time (mean ± σ):     522.8 ms ±  22.8 ms    [User: 438.9 ms, System: 130.2 ms]
      Range (min … max):   502.8 ms … 581.3 ms    10 runs
     
    Benchmark 2: GIT_TEST_INSTALLED=/home/avar/g/git/installed/bin ./t0001-init.sh --root=/dev/shm/git
      Time (mean ± σ):     473.4 ms ±  12.6 ms    [User: 406.7 ms, System: 117.5 ms]
      Range (min … max):   461.0 ms … 502.1 ms    10 runs
     
    Summary
      'GIT_TEST_INSTALLED=/home/avar/g/git/installed/bin ./t0001-init.sh --root=/dev/shm/git' ran
        1.10 ± 0.06 times faster than ' ./t0001-init.sh --root=/dev/shm/git'

And that's on a box where process spawning is really fast, and against a ramdisk.