On 2/4/2018 4:38 AM, Nguyễn Thái Ngọc Duy wrote:
read_directory() code ignores all paths named ".git" even if it's not
a valid git repository. See treat_path() for details. Since ".git" is
basically invisible to read_directory(), when we are asked to
invalidate a path that contains ".git", we can safely ignore it
because the slow path would not consider it anyway.
This helps when fsmonitor is used and we have a real ".git" repo at
worktree top. Occasionally .git/index will be updated and if the
fsmonitor hook does not filter it, untracked cache is asked to
invalidate the path ".git/index".
Without this patch, we invalidate the root directory unncessarily,
which:
- makes read_directory() fall back to slow path for root directory
(slower)
- makes the index dirty (because UNTR extension is updated). Depending
on the index size, writing it down could also be slow.
Noticed-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
---
Sorry for the resend, I forgot git@vger.
dir.c | 13 ++++++++++++-
git-compat-util.h | 2 ++
wrapper.c | 12 ++++++++++++
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/dir.c b/dir.c
index 7c4b45e30e..f8b4cabba9 100644
--- a/dir.c
+++ b/dir.c
@@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct dir_struct *dir,
if (!de)
return treat_path_fast(dir, untracked, cdir, istate, path,
baselen, pathspec);
- if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git"))
+ if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git"))
return path_none;
strbuf_setlen(path, baselen);
strbuf_addstr(path, de->d_name);
@@ -2970,8 +2970,19 @@ static int invalidate_one_component(struct untracked_cache *uc,
void untracked_cache_invalidate_path(struct index_state *istate,
const char *path)
{
+ const char *end;
+ int skipped;
+
if (!istate->untracked || !istate->untracked->root)
return;
Thank you for this patch! It's great to see people helping improve the
performance of git.
I ran a quick test and this is not sufficient to prevent the index from
getting flagged as dirty and written out to disk when fsmonitor detects
changes for files under the .git folder. What I'm seeing is that when
".git/index" is returned, the test below doesn't catch them and end
early as would be expected.
As a result, invalidate_one_component() is called which calls
invalidate_one_directory() which increments dir_invalidated counter and
the regular dirty process continues which triggers the index to be
written to disk unnecessarily.
+ if (!fspathcmp(path, ".git"))
+ return;
+ if (ignore_case)
+ skipped = skip_caseprefix(path, "/.git", &end);
+ else
+ skipped = skip_prefix(path, "/.git", &end);
+ if (skipped && (*end == '\0' || *end == '/'))
+ return;
If I replace the above lines with:
if (ignore_case)
skipped = skip_caseprefix(path, ".git", &end);
else
skipped = skip_prefix(path, ".git", &end);
Then it correctly skips _all_ files under ".git". I'm not sure if by
removing the leading slash, I'm breaking some other case but I was not
able to find where that was expected or needed. Removing the leading
slash also allows me to remove the fsmpathcmp() call as it is now redundant.
I wondered what would happen if there was some other directory named
".git" and how that would behave. With or without this patch and with
or without the untracked cache, a file "dir1/.git/foo" is ignored by git
so no change in behavior there either.
invalidate_one_component(istate->untracked, istate->untracked->root,
path, strlen(path));
}
diff --git a/git-compat-util.h b/git-compat-util.h
index 68b2ad531e..27e0b761a3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -484,6 +484,8 @@ static inline int skip_prefix(const char *str, const char *prefix,
return 0;
}
+int skip_caseprefix(const char *str, const char *prefix, const char **out);
+
/*
* If the string "str" is the same as the string in "prefix", then the "arg"
* parameter is set to the "def" parameter and 1 is returned.
diff --git a/wrapper.c b/wrapper.c
index d20356a776..bb888d9401 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -690,3 +690,15 @@ int xgethostname(char *buf, size_t len)
buf[len - 1] = 0;
return ret;
}
+
+int skip_caseprefix(const char *str, const char *prefix, const char **out)
+{
+ do {
+ if (!*prefix) {
+ *out = str;
+ return 1;
+ }
+ } while (tolower(*str++) == tolower(*prefix++));
+
+ return 0;
+}
