diff --git a/CMakeLists.txt b/CMakeLists.txt index 83e03d6cd..884c9bcf1 100644 --- a/CMakeLists.txt +++ b/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 diff --git a/include/git2/status.h b/include/git2/status.h index 6af45c7dd..ee2f33287 100644 --- a/include/git2/status.h +++ b/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 \ diff --git a/src/diff.c b/src/diff.c index caed8bf40..8b7433c62 100644 --- a/src/diff.c +++ b/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; diff --git a/src/diff.h b/src/diff.h index b2b7dba70..2e7ce0b7d 100644 --- a/src/diff.h +++ b/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); diff --git a/src/iterator.c b/src/iterator.c index 03058b956..bebdeba84 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -11,6 +11,7 @@ #include "ignore.h" #include "buffer.h" #include "submodule.h" +#include "trace.h" #include #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--; diff --git a/src/iterator.h b/src/iterator.h index 2968b8c0c..ba9c1e486 100644 --- a/src/iterator.h +++ b/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( diff --git a/src/status.c b/src/status.c index e1f8e06ae..e418cf7b6 100644 --- a/src/status.c +++ b/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 | diff --git a/src/util.h b/src/util.h index be7a16ef8..6fb2dc0f4 100644 --- a/src/util.h +++ b/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__ */ diff --git a/tests/diff/workdir.c b/tests/diff/workdir.c index 9e4608e9d..a225ebc89 100644 --- a/tests/diff/workdir.c +++ b/tests/diff/workdir.c @@ -1,21 +1,43 @@ #include "clar_libgit2.h" #include "diff_helpers.h" #include "repository.h" - -#ifdef GIT_PERF -/* access to diff usage statistics */ -# include "diff.h" -#endif +#include 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); diff --git a/tests/status/worktree.c b/tests/status/worktree.c index f51c61864..0b94fb1a0 100644 --- a/tests/status/worktree.c +++ b/tests/status/worktree.c @@ -5,6 +5,36 @@ #include "posix.h" #include "util.h" #include "path.h" +#include + +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 +}