Hi Ævar, On Tue, 23 Oct 2018, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Oct 23 2018, Johannes Schindelin wrote: > > > 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. > > The effect of what I'm suggesting here, and which my WIP patch in > <875zxtd59e....@evledraar.gmail.com> 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. Yep, you are right, we are *already* going through _() as a function, whether gettext() is initialized or not. My bad. > Perhaps some of that's expensive on Windows, but given the recent > patches by Microsoft employees to add GIT_TEST_* env options I assumed > not, but in any case it won't have anything to do with how expensive > gettext may or may not be, you'll already be paying that cost now (or > not, with NO_GETTEXT). Indeed, we want to measure performance better, and that's what those environment variables are for. Thanks, Dscho