On Tue, Oct 23, 2018 at 12:17 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > > On Tue, Oct 23 2018, Johannes Schindelin wrote: > > > Hi Ævar, > > > > On Mon, 22 Oct 2018, Ævar Arnfjörð Bjarmason wrote: > > > >> 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 > <875zxtd59e.fsf@xxxxxxxxxxxxxxxxxxx> 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