Re: [PATCH] Poison gettext with the Ook language

On Tue, Oct 23, 2018 at 12:17 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

On Tue, Oct 23 2018, Johannes Schindelin wrote:

quoted

Hi Ævar,

On Mon, 22 Oct 2018, Ævar Arnfjörð Bjarmason wrote:

quoted

So I think the only reason to keep it [GETTEXT_POISON] compile-time is
performance, but I don't think that matters. It's not like we're
printing gigabytes of _() formatted output. Everything where formatting
matters is plumbing which doesn't use this API. These messages are
always for human consumption.

Well, let's make sure that your impression is correct before going too
far. I, too, had the impression that gettext cannot possibly be expensive,
especifally in Git for Windows' settings, where we do not even ship
translations. Yet see the commit message of cc5e1bf99247 (gettext: avoid
initialization if the locale dir is not present, 2018-04-21):

      The runtime of a simple `git.exe version` call on Windows is
      currently dominated by the gettext setup, adding a whopping ~150ms
      to the ~210ms total.

I would be in favor of your change to make this a runtime option, of
course, as long as it does not affect performance greatly (in particular
on Windows, where we fight an uphill battle to make Git faster).

How expensive gettext() may or may not be isn't relevant to the
GETTEXT_POISON compile-time option.

If a user requests NO_GETTEXT, they should have zero (or near zero)
cost related to gettext. Which is true until now (the inline _()
version is expanded to pretty much no-op with the exception of NULL
string)

The effect of what I'm suggesting here, and which my WIP patch in
[ref] implements is that we'd do a
one-time getenv() for each process that prints a _() message that we
aren't doing now, and for each message call a function that would check
a boolean "are we in poison mode" static global variable.

Just don't do the getenv() inside _() even if you just do it one time.
getenv() may invalidate whatever value from the previous call. I would
not want to find out my code broke because of an getenv() inside some
innocent _()... And we should still respect NO_GETTEXT, not doing any
extra work when it's defined.
-- 
Duy