Add GIT_STATUS_OPT_UPDATE_INDEX and use trace API

This adds an option to refresh the stat cache while generating
status.  It also rips out the GIT_PERF stuff I had an makes use
of the trace API to keep statistics about what happens during diff.

CMakeLists.txt

@@ -37,7 +37,6 @@ OPTION( ANDROID				"Build for android NDK"	 				OFF )
 OPTION( USE_ICONV			"Link with and use iconv library" 		OFF )
 OPTION( USE_SSH				"Link with libssh to enable SSH support" ON )
 OPTION( VALGRIND			"Configure build for valgrind"			OFF )
-OPTION( PERF_STATS			"Internally track performance data"		OFF )
 
 IF(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 	SET( USE_ICONV ON )
@@ -353,10 +352,6 @@ IF (THREADSAFE)
 	ADD_DEFINITIONS(-DGIT_THREADS)
 ENDIF()
 
-IF (PERF_STATS)
-	ADD_DEFINITIONS(-DGIT_PERF)
-ENDIF()
-
 ADD_DEFINITIONS(-D_FILE_OFFSET_BITS=64)
 
 # Collect sourcefiles

include/git2/status.h

@@ -121,6 +121,11 @@ typedef enum {
  * - GIT_STATUS_OPT_NO_REFRESH bypasses the default status behavior of
  *   doing a "soft" index reload (i.e. reloading the index data if the
  *   file on disk has been modified outside libgit2).
+ * - GIT_STATUS_OPT_UPDATE_INDEX tells libgit2 to refresh the stat cache
+ *   in the index for files that are unchanged but have out of date stat
+ *   information in the index.  It will result in less work being done on
+ *   subsequent calls to get status.  This is mutually exclusive with the
+ *   NO_REFRESH option.
  *
  * Calling `git_status_foreach()` is like calling the extended version
  * with: GIT_STATUS_OPT_INCLUDE_IGNORED, GIT_STATUS_OPT_INCLUDE_UNTRACKED,
@@ -141,6 +146,7 @@ typedef enum {
 	GIT_STATUS_OPT_SORT_CASE_INSENSITIVELY  = (1u << 10),
 	GIT_STATUS_OPT_RENAMES_FROM_REWRITES    = (1u << 11),
 	GIT_STATUS_OPT_NO_REFRESH               = (1u << 12),
+	GIT_STATUS_OPT_UPDATE_INDEX             = (1u << 13),
 } git_status_opt_t;
 
 #define GIT_STATUS_OPT_DEFAULTS \

src/diff.c

@@ -14,6 +14,7 @@
 #include "index.h"
 #include "odb.h"
 #include "submodule.h"
+#include "trace.h"
 
 #define DIFF_FLAG_IS_SET(DIFF,FLAG) (((DIFF)->opts.flags & (FLAG)) != 0)
 #define DIFF_FLAG_ISNT_SET(DIFF,FLAG) (((DIFF)->opts.flags & (FLAG)) == 0)
@@ -554,8 +555,7 @@ int git_diff__oid_for_entry(
 	if (!entry.mode) {
 		struct stat st;
 
-		GIT_PERF_INC(diff->stat_calls);
-
+		git_trace(GIT_TRACE_TRACE, "stat=1");
 		if (p_stat(full_path.ptr, &st) < 0) {
 			error = git_path_set_error(errno, entry.path, "stat");
 			git_buf_free(&full_path);
@@ -570,8 +570,7 @@ int git_diff__oid_for_entry(
 	if (S_ISGITLINK(entry.mode)) {
 		git_submodule *sm;
 
-		GIT_PERF_INC(diff->submodule_lookups);
-
+		git_trace(GIT_TRACE_TRACE, "submodule_lookup=1");
 		if (!git_submodule_lookup(&sm, diff->repo, entry.path)) {
 			const git_oid *sm_oid = git_submodule_wd_id(sm);
 			if (sm_oid)
@@ -584,7 +583,7 @@ int git_diff__oid_for_entry(
 			giterr_clear();
 		}
 	} else if (S_ISLNK(entry.mode)) {
-		GIT_PERF_INC(diff->oid_calculations);
+		git_trace(GIT_TRACE_TRACE, "oid_calculation=1");
 		error = git_odb__hashlink(out, full_path.ptr);
 	} else if (!git__is_sizet(entry.file_size)) {
 		giterr_set(GITERR_OS, "File size overflow (for 32-bits) on '%s'",
@@ -597,7 +596,7 @@ int git_diff__oid_for_entry(
 		if (fd < 0)
 			error = fd;
 		else {
-			GIT_PERF_INC(diff->oid_calculations);
+			git_trace(GIT_TRACE_TRACE, "oid_calculation=1");
 			error = git_odb__hashfd_filtered(
 				out, fd, (size_t)entry.file_size, GIT_OBJ_BLOB, fl);
 			p_close(fd);
@@ -656,7 +655,7 @@ static int maybe_modified_submodule(
 		ign == GIT_SUBMODULE_IGNORE_ALL)
 		return 0;
 
-	GIT_PERF_INC(diff->submodule_lookups);
+	git_trace(GIT_TRACE_TRACE, "submodule_lookup=1");
 
 	if ((error = git_submodule_lookup(
 			&sub, diff->repo, info->nitem->path)) < 0) {
@@ -966,7 +965,7 @@ static int handle_unmatched_new_item(
 		delta_type = GIT_DELTA_ADDED;
 
 	else if (nitem->mode == GIT_FILEMODE_COMMIT) {
-		GIT_PERF_INC(diff->submodule_lookups);
+		git_trace(GIT_TRACE_TRACE, "submodule_lookup=1");
 
 		/* ignore things that are not actual submodules */
 		if (git_submodule_lookup(NULL, info->repo, nitem->path) != 0) {
@@ -1120,11 +1119,6 @@ int git_diff__from_iterators(
 			error = 0;
 	}
 
-	GIT_PERF_ADD(diff->stat_calls, old_iter->stat_calls);
-	GIT_PERF_ADD(diff->stat_calls, new_iter->stat_calls);
-	GIT_PERF_ADD(diff->submodule_lookups, old_iter->submodule_lookups);
-	GIT_PERF_ADD(diff->submodule_lookups, new_iter->submodule_lookups);
-
 cleanup:
 	if (!error)
 		*diff_ptr = diff;

src/diff.h

@@ -62,11 +62,6 @@ struct git_diff {
 	git_iterator_type_t old_src;
 	git_iterator_type_t new_src;
 	uint32_t diffcaps;
-#ifdef GIT_PERF
-	size_t stat_calls;
-	size_t oid_calculations;
-	size_t submodule_lookups;
-#endif
 
 	int (*strcomp)(const char *, const char *);
 	int (*strncomp)(const char *, const char *, size_t);

src/iterator.c

@@ -11,6 +11,7 @@
 #include "ignore.h"
 #include "buffer.h"
 #include "submodule.h"
+#include "trace.h"
 #include <ctype.h>
 
 #define ITERATOR_SET_CB(P,NAME_LC) do { \
@@ -1017,7 +1018,7 @@ static int fs_iterator__expand_dir(fs_iterator *fi)
 		return GIT_ENOTFOUND;
 	}
 
-	GIT_PERF_ADD(fi->base.stat_calls, ff->entries.length);
+	git_trace(GIT_TRACE_TRACE, "stat=%ld", (long)ff->entries.length);
 
 	fs_iterator__seek_frame_start(fi, ff);
 
@@ -1309,7 +1310,7 @@ static int workdir_iterator__enter_dir(fs_iterator *fi)
 		if (!S_ISDIR(entry->st.st_mode) || !strcmp(GIT_DIR, entry->path))
 			continue;
 
-		GIT_PERF_INC(fi->base.submodule_lookups);
+		git_trace(GIT_TRACE_TRACE, "submodule_lookup=1");
 		if (git_submodule__is_submodule(fi->base.repo, entry->path)) {
 			entry->st.st_mode = GIT_FILEMODE_COMMIT;
 			entry->path_len--;

src/iterator.h

@@ -53,10 +53,6 @@ struct git_iterator {
 	char *end;
 	int (*prefixcomp)(const char *str, const char *prefix);
 	unsigned int flags;
-#ifdef GIT_PERF
-	size_t stat_calls;
-	size_t submodule_lookups;
-#endif
 };
 
 extern int git_iterator_for_nothing(

src/status.c

@@ -225,6 +225,28 @@ static git_status_list *git_status_list_alloc(git_index *index)
 	return status;
 }
 
+static int status_validate_options(const git_status_options *opts)
+{
+	if (!opts)
+		return 0;
+
+	GITERR_CHECK_VERSION(opts, GIT_STATUS_OPTIONS_VERSION, "git_status_options");
+
+	if (opts->show > GIT_STATUS_SHOW_WORKDIR_ONLY) {
+		giterr_set(GITERR_INVALID, "Unknown status 'show' option");
+		return -1;
+	}
+
+	if ((opts->flags & GIT_STATUS_OPT_NO_REFRESH) != 0 &&
+		(opts->flags & GIT_STATUS_OPT_UPDATE_INDEX) != 0) {
+		giterr_set(GITERR_INVALID, "Updating index from status "
+			"is not allowed when index refresh is disabled");
+		return -1;
+	}
+
+	return 0;
+}
+
 int git_status_list_new(
 	git_status_list **out,
 	git_repository *repo,
@@ -240,11 +262,10 @@ int git_status_list_new(
 	int error = 0;
 	unsigned int flags = opts ? opts->flags : GIT_STATUS_OPT_DEFAULTS;
 
-	assert(show <= GIT_STATUS_SHOW_WORKDIR_ONLY);
-
 	*out = NULL;
 
-	GITERR_CHECK_VERSION(opts, GIT_STATUS_OPTIONS_VERSION, "git_status_options");
+	if (status_validate_options(opts) < 0)
+		return -1;
 
 	if ((error = git_repository__ensure_not_bare(repo, "status")) < 0 ||
 		(error = git_repository_index(&index, repo)) < 0)
@@ -287,6 +308,8 @@ int git_status_list_new(
 		diffopt.flags = diffopt.flags | GIT_DIFF_RECURSE_IGNORED_DIRS;
 	if ((flags & GIT_STATUS_OPT_EXCLUDE_SUBMODULES) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_IGNORE_SUBMODULES;
+	if ((flags & GIT_STATUS_OPT_UPDATE_INDEX) != 0)
+		diffopt.flags = diffopt.flags | GIT_DIFF_UPDATE_INDEX;
 
 	if ((flags & GIT_STATUS_OPT_RENAMES_FROM_REWRITES) != 0)
 		findopt.flags = findopt.flags |

src/util.h

@@ -436,12 +436,4 @@ GIT_INLINE(double) git__timer(void)
 
 #endif
 
-#ifdef GIT_PERF
-#	define GIT_PERF_INC(counter) (counter)++
-#	define GIT_PERF_ADD(counter,val) (counter) += (val)
-#else
-#	define GIT_PERF_INC(counter) 0
-#	define GIT_PERF_ADD(counter,val) 0
-#endif
-
 #endif /* INCLUDE_util_h__ */

tests/diff/workdir.c

 #include "clar_libgit2.h"
 #include "diff_helpers.h"
 #include "repository.h"
-
-#ifdef GIT_PERF
-/* access to diff usage statistics */
-#	include "diff.h"
-#endif
+#include <git2/trace.h>
 
 static git_repository *g_repo = NULL;
 
+static struct {
+	size_t stat_calls;
+	size_t oid_calcs;
+	size_t submodule_lookups;
+} g_diff_perf;
+
+static void add_stats(git_trace_level_t level, const char *msg)
+{
+	const char *assign = strchr(msg, '=');
+
+	GIT_UNUSED(level);
+
+	if (!assign)
+		return;
+
+	if (!strncmp("stat", msg, (assign - msg)))
+		g_diff_perf.stat_calls += atoi(assign + 1);
+	else if (!strncmp("submodule_lookup", msg, (assign - msg)))
+		g_diff_perf.submodule_lookups += atoi(assign + 1);
+	else if (!strncmp("oid_calculation", msg, (assign - msg)))
+		g_diff_perf.oid_calcs += atoi(assign + 1);
+}
+
 void test_diff_workdir__initialize(void)
 {
+	memset(&g_diff_perf, 0, sizeof(g_diff_perf));
+	cl_git_pass(git_trace_set(GIT_TRACE_TRACE, add_stats));
 }
 
 void test_diff_workdir__cleanup(void)
 {
 	cl_git_sandbox_cleanup();
+	cl_git_pass(git_trace_set(0, NULL));
 }
 
 void test_diff_workdir__to_index(void)
@@ -64,11 +86,11 @@ void test_diff_workdir__to_index(void)
 		cl_assert_equal_i(4, exp.line_adds);
 		cl_assert_equal_i(5, exp.line_dels);
 
-#ifdef GIT_PERF
+#ifdef GIT_TRACE
 		cl_assert_equal_sz(
-			13 /* in root */ + 3 /* in subdir */, diff->stat_calls);
-		cl_assert_equal_sz(5, diff->oid_calculations);
-		cl_assert_equal_sz(1, diff->submodule_lookups);
+			13 /* in root */ + 3 /* in subdir */, g_diff_perf.stat_calls);
+		cl_assert_equal_sz(5, g_diff_perf.oid_calcs);
+		cl_assert_equal_sz(1, g_diff_perf.submodule_lookups);
 #endif
 	}
 
@@ -1525,6 +1547,8 @@ static void basic_diff_status(git_diff **out, const git_diff_options *opts)
 {
 	diff_expects exp;
 
+	memset(&g_diff_perf, 0, sizeof(g_diff_perf));
+
 	cl_git_pass(git_diff_index_to_workdir(out, g_repo, NULL, opts));
 
 	memset(&exp, 0, sizeof(exp));
@@ -1558,10 +1582,10 @@ void test_diff_workdir__can_update_index(void)
 	opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED;
 
 	basic_diff_status(&diff, &opts);
-#ifdef GIT_PERF
-	cl_assert_equal_sz(diff->stat_calls, 13 + 3);
-	cl_assert_equal_sz(diff->oid_calculations, 5);
-	cl_assert_equal_sz(diff->submodule_lookups, 1);
+#ifdef GIT_TRACE
+	cl_assert_equal_sz(13 + 3, g_diff_perf.stat_calls);
+	cl_assert_equal_sz(5, g_diff_perf.oid_calcs);
+	cl_assert_equal_sz(1, g_diff_perf.submodule_lookups);
 #endif
 
 	git_diff_free(diff);
@@ -1570,10 +1594,10 @@ void test_diff_workdir__can_update_index(void)
 	opts.flags |= GIT_DIFF_UPDATE_INDEX;
 
 	basic_diff_status(&diff, &opts);
-#ifdef GIT_PERF
-	cl_assert_equal_sz(diff->stat_calls, 13 + 3);
-	cl_assert_equal_sz(diff->oid_calculations, 5);
-	cl_assert_equal_sz(diff->submodule_lookups, 1);
+#ifdef GIT_TRACE
+	cl_assert_equal_sz(13 + 3, g_diff_perf.stat_calls);
+	cl_assert_equal_sz(5, g_diff_perf.oid_calcs);
+	cl_assert_equal_sz(1, g_diff_perf.submodule_lookups);
 #endif
 
 	git_diff_free(diff);
@@ -1581,10 +1605,10 @@ void test_diff_workdir__can_update_index(void)
 	/* now if we do it again, we should see fewer OID calculations */
 
 	basic_diff_status(&diff, &opts);
-#ifdef GIT_PERF
-	cl_assert_equal_sz(diff->stat_calls, 13 + 3);
-	cl_assert_equal_sz(diff->oid_calculations, 0); /* Yay */
-	cl_assert_equal_sz(diff->submodule_lookups, 1);
+#ifdef GIT_TRACE
+	cl_assert_equal_sz(13 + 3, g_diff_perf.stat_calls);
+	cl_assert_equal_sz(0, g_diff_perf.oid_calcs); /* Yay */
+	cl_assert_equal_sz(1, g_diff_perf.submodule_lookups);
 #endif
 
 	git_diff_free(diff);

tests/status/worktree.c

@@ -5,6 +5,36 @@
 #include "posix.h"
 #include "util.h"
 #include "path.h"
+#include <git2/trace.h>
+
+static struct {
+	size_t stat_calls;
+	size_t oid_calcs;
+	size_t submodule_lookups;
+} g_diff_perf;
+
+static void add_stats(git_trace_level_t level, const char *msg)
+{
+	const char *assign = strchr(msg, '=');
+
+	GIT_UNUSED(level);
+
+	if (!assign)
+		return;
+
+	if (!strncmp("stat", msg, (assign - msg)))
+		g_diff_perf.stat_calls += atoi(assign + 1);
+	else if (!strncmp("submodule_lookup", msg, (assign - msg)))
+		g_diff_perf.submodule_lookups += atoi(assign + 1);
+	else if (!strncmp("oid_calculation", msg, (assign - msg)))
+		g_diff_perf.oid_calcs += atoi(assign + 1);
+}
+
+void test_status_worktree__initialize(void)
+{
+	memset(&g_diff_perf, 0, sizeof(g_diff_perf));
+	cl_git_pass(git_trace_set(GIT_TRACE_TRACE, add_stats));
+}
 
 /**
  * Cleanup
@@ -15,6 +45,7 @@
 void test_status_worktree__cleanup(void)
 {
 	cl_git_sandbox_cleanup();
+	cl_git_pass(git_trace_set(0, NULL));
 }
 
 /**
@@ -40,11 +71,15 @@ void test_status_worktree__whole_repository(void)
 	cl_assert_equal_i(0, counts.wrong_sorted_path);
 }
 
-void assert_show(const int entry_counts, const char *entry_paths[],
-				 const unsigned int entry_statuses[], git_status_show_t show)
+void assert_show(
+	const int entry_counts,
+	const char *entry_paths[],
+	const unsigned int entry_statuses[],
+	git_repository *repo,
+	git_status_show_t show,
+	unsigned int extra_flags)
 {
 	status_entry_counts counts;
-	git_repository *repo = cl_git_sandbox_init("status");
 	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
 
 	memset(&counts, 0x0, sizeof(status_entry_counts));
@@ -52,7 +87,7 @@ void assert_show(const int entry_counts, const char *entry_paths[],
 	counts.expected_paths = entry_paths;
 	counts.expected_statuses = entry_statuses;
 
-	opts.flags = GIT_STATUS_OPT_DEFAULTS;
+	opts.flags = GIT_STATUS_OPT_DEFAULTS | extra_flags;
 	opts.show = show;
 
 	cl_git_pass(
@@ -67,19 +102,19 @@ void assert_show(const int entry_counts, const char *entry_paths[],
 void test_status_worktree__show_index_and_workdir(void)
 {
 	assert_show(entry_count0, entry_paths0, entry_statuses0,
-		GIT_STATUS_SHOW_INDEX_AND_WORKDIR);
+		cl_git_sandbox_init("status"), GIT_STATUS_SHOW_INDEX_AND_WORKDIR, 0);
 }
 
 void test_status_worktree__show_index_only(void)
 {
 	assert_show(entry_count5, entry_paths5, entry_statuses5,
-		GIT_STATUS_SHOW_INDEX_ONLY);
+		cl_git_sandbox_init("status"), GIT_STATUS_SHOW_INDEX_ONLY, 0);
 }
 
 void test_status_worktree__show_workdir_only(void)
 {
 	assert_show(entry_count6, entry_paths6, entry_statuses6,
-		GIT_STATUS_SHOW_WORKDIR_ONLY);
+		cl_git_sandbox_init("status"), GIT_STATUS_SHOW_WORKDIR_ONLY, 0);
 }
 
 /* this test is equivalent to t18-status.c:statuscb1 */
@@ -877,3 +912,45 @@ void test_status_worktree__long_filenames(void)
 	cl_assert_equal_i(0, counts.wrong_sorted_path);
 }
 
+/* The update stat cache tests mostly just mirror other tests and try
+ * to make sure that updating the stat cache doesn't change the results
+ * while reducing the amount of work that needs to be done
+ */
+
+void test_status_worktree__update_stat_cache_0(void)
+{
+	git_repository *repo = cl_git_sandbox_init("status");
+
+	assert_show(entry_count0, entry_paths0, entry_statuses0,
+		repo, GIT_STATUS_SHOW_INDEX_AND_WORKDIR, 0);
+
+#ifdef GIT_TRACE
+	cl_assert_equal_sz(13 + 3, g_diff_perf.stat_calls);
+	cl_assert_equal_sz(5, g_diff_perf.oid_calcs);
+	cl_assert_equal_sz(1, g_diff_perf.submodule_lookups);
+
+	memset(&g_diff_perf, 0, sizeof(g_diff_perf));
+#endif
+
+	assert_show(entry_count0, entry_paths0, entry_statuses0,
+		repo, GIT_STATUS_SHOW_INDEX_AND_WORKDIR, GIT_STATUS_OPT_UPDATE_INDEX);
+
+#ifdef GIT_TRACE
+	cl_assert_equal_sz(13 + 3, g_diff_perf.stat_calls);
+	cl_assert_equal_sz(5, g_diff_perf.oid_calcs);
+	cl_assert_equal_sz(1, g_diff_perf.submodule_lookups);
+
+	memset(&g_diff_perf, 0, sizeof(g_diff_perf));
+#endif
+
+	assert_show(entry_count0, entry_paths0, entry_statuses0,
+		repo, GIT_STATUS_SHOW_INDEX_AND_WORKDIR, 0);
+
+#ifdef GIT_TRACE
+	cl_assert_equal_sz(13 + 3, g_diff_perf.stat_calls);
+	cl_assert_equal_sz(0, g_diff_perf.oid_calcs);
+	cl_assert_equal_sz(1, g_diff_perf.submodule_lookups);
+
+	memset(&g_diff_perf, 0, sizeof(g_diff_perf));
+#endif
+}