fetch: skip fsmonitor initialization - #2193
Open
spkrka wants to merge 1 commit into
Open
Conversation
Fetching does not need to monitor the working tree for filesystem changes. While fetch itself does not read the index, fetching with submodule recursion triggers config_from_gitmodules() which calls repo_read_index(). This in turn activates fsmonitor via tweak_fsmonitor() in post_read_index_from(), causing an unnecessary and potentially expensive filesystem monitor refresh. Introduce command_requires_fsmonitor, analogous to the existing command_requires_full_index setting, defaulting to 1 (enabled). Commands that do not need filesystem monitoring can set it to 0 to skip fsmonitor activation. Set command_requires_fsmonitor to 0 in cmd_fetch, since fetch only needs to read remote refs and download objects; it does not examine the working tree. Skipping fsmonitor is never harmful for operations that do not need it. On a small repository (~7k files) with a healthy fsmonitor daemon, the overhead is negligible and lost in network latency. On a large repository where the daemon has exhausted inotify watches and falls back to a full filesystem scan, this reduces no-op fetch time from 9.1s to 2.6s (mean of 15 runs, measured on Linux). Signed-off-by: Kristofer Karlsson <krka@spotify.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When fetching in a repository with fsmonitor enabled, submodule
recursion triggers config_from_gitmodules() which reads the index.
Reading the index activates fsmonitor via tweak_fsmonitor() in
post_read_index_from(), causing an unnecessary daemon query. On
repositories where inotify watches are exhausted and the daemon
falls back to a full filesystem scan, this adds several seconds
to every fetch.
This patch introduces command_requires_fsmonitor (analogous to
command_requires_full_index) and sets it to 0 in cmd_fetch.
Benchmark results (mean of 15 runs, Linux, built-in fsmonitor
daemon):
git.git (~7k files, healthy daemon):
before: 3.3s after: 3.3s (no change, network-dominated)
Large repo (~500k files, exhausted inotify watches):
before: 9.1s after: 2.6s (3.5x faster)
Skipping fsmonitor is never harmful for operations that do not
examine the working tree.
An alternative considered was preventing the index read from
activating fsmonitor in the first place, but tweak_fsmonitor()
is called unconditionally from post_read_index_from() after every
index read -- that is the right design for commands like status,
diff, and add that need fsmonitor as soon as the index is loaded.
The other options (making config_from_gitmodules() avoid the index
read, or adding a "read index without side effects" API) would be
larger changes for the same result. The command_requires_fsmonitor
flag is the least invasive approach and follows the established
pattern.
cc: Jeff Hostetler jeffhostetler@github.com