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

From: Felipe Contreras <[email-masked]>
Date: Fri, 28 May 2021 10:55:15 -0500

Ævar Arnfjörð Bjarmason wrote:
> 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