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

On Fri, Aug 7, 2026 at 5:17 PM Junio C Hamano <[email protected]> wrote:
>
> "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.

[replied to SZEDER down-thread]

>
> > 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?

TL;DR yes, but the patch series doesn't currently put the member in
repo_settings (repo_config_values). End of mail contains some
commentary there; folks from <[email protected]> cc'd.

I did some benchmarking on linux.git @ 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d.

Brand-new worktree, without refreshing the index:

hyperfine -N --warmup=10 \
-n core.useNanosec=false ~c/'git/build/bin-wrappers/git diff' \
-n core.useNanosec=true  ~c/'git/build/bin-wrappers/git -c
core.useNanosec=true diff' \
-n v2.55.0_USE_NSEC_disabled ~c/'perf-test/build/bin-wrappers/git diff'
Benchmark 1: core.useNanosec=false
  Time (mean ± σ):     853.7 ms ±  23.2 ms    [User: 823.2 ms, System: 159.3 ms]
  Range (min … max):   839.5 ms … 904.7 ms    10 runs

  Warning: Statistical outliers were detected. Consider re-running
this benchmark on a quiet system without any interferences from other
programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: core.useNanosec=true
  Time (mean ± σ):      20.4 ms ±   1.5 ms    [User: 41.0 ms, System: 102.2 ms]
  Range (min … max):    17.6 ms …  24.8 ms    122 runs

Benchmark 3: v2.55.0_USE_NSEC_disabled
  Time (mean ± σ):     839.2 ms ±  12.7 ms    [User: 796.8 ms, System: 158.8 ms]
  Range (min … max):   830.5 ms … 864.4 ms    10 runs

  Warning: Statistical outliers were detected. Consider re-running
this benchmark on a quiet system without any interferences from other
programs. It might help to use the '--warmup' or '--prepare' options.

Summary
  core.useNanosec=true ran
   41.06 ± 3.03 times faster than v2.55.0_USE_NSEC_disabled
   41.77 ± 3.22 times faster than core.useNanosec=false

Same worktree after "git update-index --refresh":

hyperfine -N --warmup=10 \
-n core.useNanosec=false ~c/'git/build/bin-wrappers/git diff' \
-n core.useNanosec=true  ~c/'git/build/bin-wrappers/git -c
core.useNanosec=true diff' \
-n v2.55.0_USE_NSEC_disabled ~c/'perf-test/build/bin-wrappers/git diff'
Benchmark 1: core.useNanosec=false
  Time (mean ± σ):      20.8 ms ±   2.1 ms    [User: 41.8 ms, System: 102.9 ms]
  Range (min … max):    17.4 ms …  26.4 ms    126 runs

Benchmark 2: core.useNanosec=true
  Time (mean ± σ):      20.0 ms ±   1.0 ms    [User: 40.4 ms, System: 101.8 ms]
  Range (min … max):    18.0 ms …  23.6 ms    158 runs

Benchmark 3: v2.55.0_USE_NSEC_disabled
  Time (mean ± σ):      19.2 ms ±   1.1 ms    [User: 27.3 ms, System: 100.2 ms]
  Range (min … max):    16.9 ms …  23.4 ms    160 runs

Summary
  v2.55.0_USE_NSEC_disabled ran
    1.04 ± 0.08 times faster than core.useNanosec=true
    1.08 ± 0.13 times faster than core.useNanosec=false

So yeah, when we don't need the nanosec timings, this ends up minutely
slower than without it. When I apply the attached patch (sorry, GMail)
on top to poke through
istate->repo->config_values_private_.use_nanosec:

New worktree, no index refresh:

hyperfine -N --warmup=10 \
-n v2_core.useNanosec=false ~c/'git/build/bin-wrappers/git diff' \
-n v2_core.useNanosec=true  ~c/'git/build/bin-wrappers/git -c
core.useNanosec=true diff' \
-n v2.55.0_USE_NSEC_disabled ~c/'perf-test/build/bin-wrappers/git diff'
Benchmark 1: v2_core.useNanosec=false
  Time (mean ± σ):     148.0 ms ±   2.8 ms    [User: 142.5 ms, System: 124.0 ms]
  Range (min … max):   144.3 ms … 155.3 ms    20 runs

Benchmark 2: v2_core.useNanosec=true
  Time (mean ± σ):      21.2 ms ±   2.0 ms    [User: 27.6 ms, System: 101.4 ms]
  Range (min … max):    17.5 ms …  28.8 ms    123 runs

Benchmark 3: v2.55.0_USE_NSEC_disabled
  Time (mean ± σ):     148.4 ms ±   8.6 ms    [User: 141.0 ms, System: 122.8 ms]
  Range (min … max):   140.9 ms … 179.6 ms    21 runs

Summary
  v2_core.useNanosec=true ran
    7.00 ± 0.67 times faster than v2_core.useNanosec=false
    7.01 ± 0.77 times faster than v2.55.0_USE_NSEC_disabled

(We can see the raciness in the variability of the timings, neat)

After "git update-index --refresh":

hyperfine -N --warmup=10 \
-n v2_core.useNanosec=false ~c/'git/build/bin-wrappers/git diff' \
-n v2_core.useNanosec=true  ~c/'git/build/bin-wrappers/git -c
core.useNanosec=true diff' \
-n v2.55.0_USE_NSEC_disabled ~c/'perf-test/build/bin-wrappers/git diff'
Benchmark 1: v2_core.useNanosec=false
  Time (mean ± σ):      20.8 ms ±   2.6 ms    [User: 27.9 ms, System: 103.8 ms]
  Range (min … max):    17.2 ms …  29.1 ms    132 runs

Benchmark 2: v2_core.useNanosec=true
  Time (mean ± σ):      19.7 ms ±   1.5 ms    [User: 29.2 ms, System: 100.0 ms]
  Range (min … max):    17.0 ms …  28.2 ms    170 runs

Benchmark 3: v2.55.0_USE_NSEC_disabled
  Time (mean ± σ):      19.7 ms ±   1.6 ms    [User: 27.8 ms, System: 99.1 ms]
  Range (min … max):    16.8 ms …  25.0 ms    154 runs

Summary
  v2_core.useNanosec=true ran
    1.00 ± 0.11 times faster than v2.55.0_USE_NSEC_disabled
    1.05 ± 0.15 times faster than v2_core.useNanosec=false

Back down to being on-par with original code. So that's good. The next
version will include some variant that reads a struct member instead
of going through repo_config_get_bool().

But which? Reading the private_ member is obviously wrong; I suppose
I'm supposed to use repo_config_values() there. Or, rework the series
to put this member in repo_settings. I think I originally assumed that
struct is for things that are settings that aren't configured by
git-config, but… now I'm not sure. Looking at prepare_repo_settings()
shows lots of repo_cfg_*() calls. So I think I see how to adapt to
using repo_settings,

Patrick, Junio, and Tian had a brief discussion in
<[email protected]> about the split creating confusion. I don't
really want to wait for it to settle to land this change, but we might
want to work together on identifying the best path forward for
core.useNanosec :)

I don't suppose it really matters to me which struct I put the member
in. As I said, v2 will definitely fix the hot path lookup here. Just a
matter of input on which struct we want to use this time, I guess.

-- 
D. Ben Knoble

diff --git i/read-cache.c w/read-cache.c
index 297646c357..4bb5f466a1 100644
--- i/read-cache.c
+++ w/read-cache.c
@@ -353,8 +353,9 @@ 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);
+	/* supposed to use repo_config_values(), probably?
+	 * or we should move this member to struct repo_settings */
+	int use_nsec = istate->repo->config_values_private_.use_nanosec;
 
 	return (istate->timestamp.sec &&
 		/* nanosecond timestamped files can also be racy! */