Re: [PATCH 3/3] core: convert build-time USE_NSEC into runtime core.useNanosec

"D. Ben Knoble" <[email protected]> writes:

> Racy Git problems persist today, manifesting themselves in the
> performance of commands like "git diff" in new worktrees [1]. We have
> long had a build knob "USE_NSEC" to tell Git to use in-core nanosecond
> precision when available, which mitigates most if not all racy issues,
> but most builds we know about it don't use it. In part, that's because
> someone distributing Git can't safely enable it at compile-time if they
> don't know exactly what platforms their distribution will be used on.
>
> [1]: https://lore.kernel.org/git/CALnO6CADMJSixqYvL1Yo8qKX5rWhKQ+2OoSEuPUh-yoeK9TseQ@mail.gmail.com
>
> These days, most platforms are likely to be safe for the USE_NSEC code.
> Regardless, we want to give users the ability to benefit from it. This
> requires exposing the compile-time gated code as a runtime option.
>
> In addition, update the Racy Git documentation and other mentions of
> USE_NSEC in the code.
>
> Best-viewed-with: --ignore-space-change

Don't do this.  It probably is helpful to have something like that
below the three-dash lines, though.

> Signed-off-by: D. Ben Knoble <[email protected]>
> ---

> diff --git a/environment.c b/environment.c
> index 6676e6f5ae..e6a50060e8 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -571,6 +571,11 @@ int git_default_core_config(const char *var, const char *value,
>  		return 0;
>  	}
>  
> +	if (!strcmp(var, "core.usenanosec")) {
> +		cfg->use_nanosec = git_config_bool(var, value);
> +		return 0;
> +	}

OK.

> diff --git a/read-cache.c b/read-cache.c
> index 6c449f393d..297646c357 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -353,15 +353,16 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
>  static int is_racy_stat(const struct index_state *istate,
>  			const struct stat_data *sd)
>  {
> +	int use_nsec = 0;
> +	repo_config_get_bool(the_repository, "core.useNanosec", &use_nsec);

Yeek.  Isn't this a relatively hot code path?  If it is, it is
criminal to force string parsing and matching like this, every time
somebody calls the function.

Doesn't istate know what repository it is working with and in there
you should be able find its repo_settings struct cheaply, no?