Skip to content

builtin/add: enable fscache around repo_read_index_preload#899

Merged
dscho merged 2 commits intomicrosoft:vfs-2.53.0from
tonybaloney:wsl-fscache-add
Apr 28, 2026
Merged

builtin/add: enable fscache around repo_read_index_preload#899
dscho merged 2 commits intomicrosoft:vfs-2.53.0from
tonybaloney:wsl-fscache-add

Conversation

@tonybaloney
Copy link
Copy Markdown

Trace2 + GIT_TRACE_FSCACHE evidence on Windows ARM64 (Snapdragon X Elite, ReFS Dev Drive) shows that the heaviest lstat-bound work in git add happens inside repo_read_index_preload(), which currently runs before enable_fscache() is called. Moving the enable up so the preload phase is wrapped lets the existing batched NtQueryDirectoryFile cache cover the bulk of the lstat traffic. This patch gave me a ~30% performance improvement on a large git repo with a batched add.

Also at the end of cmd_add(): the cleanup site called enable_fscache(0) again instead of disable_fscache(), leaking the refcount.

dscho
dscho previously approved these changes Apr 28, 2026
Copy link
Copy Markdown
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will split the commit a bit, and "forge" your SOB.

Comment thread builtin/add.c
enable_fscache(0);
/* We do not really re-read the index but update the up-to-date flags */
preload_index(repo->index, &pathspec, 0);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this empty line.

Comment thread builtin/add.c
dir_clear(&dir);
clear_pathspec(&pathspec);
enable_fscache(0);
disable_fscache();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. This was missed in 6378e39.

Comment thread builtin/add.c
die_in_unpopulated_submodule(repo->index, prefix);
die_path_inside_submodule(repo->index, &pathspec);

enable_fscache(0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This basically moves enable_fscache(0); around, so that the repo_read_index_preload() call benefits from FSCache. This should be a fixup for 6d30b7e.

That commit is a heavily adapted version of 38df35b. It was I who introduced this bug, in 19b6958.

Anthony Shaw added 2 commits April 28, 2026 09:40
Trace2 + GIT_TRACE_FSCACHE evidence on Windows ARM64 (Snapdragon X
Elite, ReFS Dev Drive) shows that the heaviest `lstat()`-bound work in
`git add` happens inside `repo_read_index_preload()`, which currently
runs *before* `enable_fscache()` is called. Moving the enable up so the
preload phase is wrapped lets the existing batched
`NtQueryDirectoryFile()` cache cover the bulk of the lstat traffic.

Signed-off-by: Anthony Shaw <anthonyshaw@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Fix a copy-paste bug at the end of `cmd_add()`: the cleanup site called
`enable_fscache(0)` again instead of `disable_fscache()`, leaking the
refcount. Harmless in a one-shot process today, but it confuses the
matching enable/disable contract that callers and reviewers expect.

Signed-off-by: Anthony Shaw <anthonyshaw@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Copy Markdown
Member

dscho commented Apr 28, 2026

I will fix the fedora-breaking-changes failure first, then force-push a slightly rewritten version of this branch, and then merge and deploy to a pre-release.

@dscho dscho force-pushed the wsl-fscache-add branch from 1bf22fb to add7a58 Compare April 28, 2026 13:28
@dscho dscho merged commit 9ac4846 into microsoft:vfs-2.53.0 Apr 28, 2026
310 of 316 checks passed
dscho added a commit that referenced this pull request Apr 28, 2026
…`vfs-2.54.0` (#902)

This is a companion of #899 and
git-for-windows#6216.

Trace2 + GIT_TRACE_FSCACHE evidence on Windows ARM64 (Snapdragon X
Elite, ReFS Dev Drive) shows that the heaviest lstat-bound work in git
add happens inside repo_read_index_preload(), which currently runs
before enable_fscache() is called. Moving the enable up so the preload
phase is wrapped lets the existing batched NtQueryDirectoryFile cache
cover the bulk of the lstat traffic. This patch gave me a ~30%
performance improvement on a large git repo with a batched add.

Also at the end of cmd_add(): the cleanup site called enable_fscache(0)
again instead of disable_fscache(), leaking the refcount.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants