Re: [PATCH 0/9] send-email: various optimizations to speed up by >2x

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: May 13, 2021, 07:37 UTC
Message-ID: <87sg2rrsg1.fsf@evledraar.gmail.com>
In-Reply-To: <YJxmmyM9/3KBFPTg@coredump.intra.peff.net>

On Wed, May 12 2021, Jeff King wrote:

Show 17 quoted lines

> On Wed, May 12, 2021 at 03:48:16PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> As noted in the subject this speeds up git-send-email invocations by
>> ~2x or more, and brings the very slow t9001 test from running in ~26s
>> on my box to ~12s. It's no longer consistently the slowest test I run.
>> 
>> This is basically done in two ways: We lazily invoke "git config" to
>> get config, before it's very eager, and deferring Perl compilation
>> with s/use/require/g.
>
> Splitting my reply, since the other one got deep into test-suite timing
> details.
>
> The techniques here look overall pretty reasonable. I think the module
> lazy-loading makes the overall code a _little_ uglier, but IMHO the
> speedup you're getting is worth it (I was surprised how much of the
> improvement comes from that versus avoiding git-config subprocesses).

Yeah, it's mostly uglier, but I think it's worth it. Some parts are better afterwards though, i.e. the SOME_RARE_BARE_WORD is now Module::It::Is::In::SOME_RARE_BARE_WORD, which makes it easier to understand where it's from.

> My only concern is changing the interface of Git::config_regexp() in the
> final patch. Do we need to have a config_regexp_with_values() to avoid
> breaking third-party users of the module?

As noted in 3/9 I don't think we need to worry about it, it's recently introduced (a few months) API in Git.pm for send-email itself. I think we can just change it.

In general I think it's unfortunate that we have (at least in principle) a "public by default" module like Git.pm that's mostly for our own use.

This series doesn't try to deal with that in general at all, I'm somewhat of the opinion that we should just fork it at this point. I.e. have a Git.pm we freeze in time, and a Git/Ours.pm that's going to be the private API.

I stopped with these optimizations at the point of refactoring away Error.pm, which is a large contributor to compilation time, but as long as it's a public API that can't be done without changing the public API. If all we needed to worry about was send-email, git-svn etc. just changing it to Perl-native exceptions would be trivial.