Re: [PATCH] commit: Optimize number of lstat() calls
From: Junio C Hamano <hidden>
Date: 2017-02-03 23:38:49
"Gumbel, Matthew K" [off-list ref] writes:
When making a --only commit, original behavior was to do a full cache update for the purposes of giving the pre-commit hook an up-to-date set of stat data. That would result in long runtime for git-commit in a big repo on NFS (>60s for a 54k-file repo). With this change, when doing a --only commit and no pre-commit hook is present, the cache update is skipped since it is known a priori which files are to be committed.
Did you determine that "pre-commit" hook is the only thing that needs the special case and if so how? With that stated in the log message, we would feel safe to take this patch.
quoted hunk
Signed-off-by: Matthew K. Gumbel <redacted> --- builtin/commit.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)diff --git a/builtin/commit.c b/builtin/commit.c index 2de5f6c..1df3d71 100644 --- a/builtin/commit.c +++ b/builtin/commit.c@@ -470,7 +470,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefixhold_locked_index(&index_lock, LOCK_DIE_ON_ERROR); add_remove_files(&partial); - refresh_cache(REFRESH_QUIET); + if (find_hook("pre-commit")) { + refresh_cache(REFRESH_QUIET); + } update_main_cache_tree(WRITE_TREE_SILENT); if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK)) die(_("unable to write new_index file"));
I wonder why this patch is full of 0xa0 bytes instead of spaces. Somebody between your editor and mailing list is munging your patches?
quoted hunk
@@ -482,7 +484,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefixcreate_base_index(current_head); add_remove_files(&partial); - refresh_cache(REFRESH_QUIET); + if (find_hook("pre-commit")) { + refresh_cache(REFRESH_QUIET); + } if (write_locked_index(&the_index, &false_lock, CLOSE_LOCK)) die(_("unable to write temporary index file"));
I'd prefer if you introduced a small helper function, name it
can_cheat_on_refreshing() or something, and encapsulate the call to
find_hook("pre-commit") inside it---if there are other corner cases
you didn't think of, or if in the future more hooks are added that
need their incoming index file already refreshed, we do not want to
update these two places---rather we'd want to limit the need for
updating to the single helper function.