RE: [PATCH v5 10/13] send-email: lazily load modules for a big speedup

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 28, 2021, 15:55 UTC
Message-ID: <60b112638fc47_4b03220885@natae.notmuch>
In-Reply-To: <patch-10.13-9f21bc6e6f2-20210528T092228Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> Optimize the time git-send-email takes to do even the simplest of
> things (such as serving up "-h") from around ~150ms to ~80ms-~90ms by
> lazily loading the modules it requires.
> 
> Before this change Devel::TraceUse would report 99/97 used modules
> under NO_GETTEXT=[|Y], respectively. Now it's 52/37. It now takes ~15s
> to run t9001-send-email.sh, down from ~20s.
> 
> Changing File::Spec::Functions::{catdir,catfile} to invoking class
> methods on File::Spec itself is idiomatic. See [1] for a more
> elaborate explanation, the resulting code behaves the same way, just
> without the now-pointless function wrapper.

I would reference `man File::Spec` rather than an email.

And while this change makes sense, I think it should be split in two.

Instead of doing:

  -use Term::ANSIColor;
  -print color("reset"), "\n";
  +require Term::ANSIColor;
  +print Term::ANSIColor::color("reset"), "\n";

We could do this in one patch:

  -print color("reset"), "\n";
  +print Term::ANSIColor::color("reset"), "\n";

That is just no-op noise that we can mostly ignore in the review.

Then the actual change to require Term::ANSIColor selectively would be much simpler to see.

Cheers.

-- 
Felipe Contreras