Re: [PATCH 3/3] core: convert build-time USE_NSEC into runtime core.useNanosec
- From
D. Ben Knoble <ben.knoble@gmail.com>
- Date
- Aug 10, 2026, 12:27 UTC
- Message-ID
- <CALnO6CBm4g27mWBvD9m6yL0e5YZu3M9_zcUeLZk7QwTgnxMLQA@mail.gmail.com>
- In-Reply-To
- <xmqqv79ld40c.fsf@gitster.g>
On Fri, Aug 7, 2026 at 5:17 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 24 quoted lines
> > "D. Ben Knoble" <ben.knoble@gmail.com> 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]
Show 36 quoted lines
>
> > Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
> > ---
>
> > 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 <anlmwaEtwcCPse1N@pks.im> 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=falseSo 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=falseBack 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 <anlmwaEtwcCPse1N@pks.im> 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! */