From b23b112dfe8eceb39eaaea2d5e60d971c4371aa0 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 29 Apr 2014 11:29:49 -0700 Subject: Add payloads, bitmaps to trace API This is a proposed adjustment to the trace APIs. This makes the trace levels into a bitmask so that they can be selectively enabled and adds a callback-level payload, plus a message-level payload. This makes it easier for me to a GIT_TRACE_PERF callbacks that are simply bypassed if the PERF level is not set. --- include/git2/trace.h | 43 ++++++++++++++++++++++++++++++------------- src/diff.c | 12 ++++++------ src/iterator.c | 4 ++-- src/trace.c | 15 +++++++++------ src/trace.h | 19 +++++++++++-------- tests/diff/diff_helpers.c | 19 +++++++++++++++++++ tests/diff/diff_helpers.h | 14 ++++++++++++++ tests/diff/workdir.c | 29 ++++------------------------- tests/status/worktree.c | 32 +++++--------------------------- tests/trace/trace.c | 39 ++++++++++++++++++++++----------------- 10 files changed, 122 insertions(+), 104 deletions(-) diff --git a/include/git2/trace.h b/include/git2/trace.h index f9b4d6ff6..867b34612 100644 --- a/include/git2/trace.h +++ b/include/git2/trace.h @@ -20,47 +20,64 @@ GIT_BEGIN_DECL /** - * Available tracing levels. When tracing is set to a particular level, - * callers will be provided tracing at the given level and all lower levels. + * Available tracing messages. Each tracing level can be enabled + * independently or pass GIT_TRACE_ALL to enable all levels. */ typedef enum { /** No tracing will be performed. */ - GIT_TRACE_NONE = 0, + GIT_TRACE_NONE = 0x0000u, + + /** All tracing messages will be sent. */ + GIT_TRACE_ALL = 0xFFFFu, /** Severe errors that may impact the program's execution */ - GIT_TRACE_FATAL = 1, + GIT_TRACE_FATAL = 0x0001u, /** Errors that do not impact the program's execution */ - GIT_TRACE_ERROR = 2, + GIT_TRACE_ERROR = 0x0002u, + GIT_TRACE_ERROR_AND_BELOW = 0x0003u, /** Warnings that suggest abnormal data */ - GIT_TRACE_WARN = 3, + GIT_TRACE_WARN = 0x0004u, + GIT_TRACE_WARN_AND_BELOW = 0x0007u, /** Informational messages about program execution */ - GIT_TRACE_INFO = 4, + GIT_TRACE_INFO = 0x0008u, + GIT_TRACE_INFO_AND_BELOW = 0x000Fu, /** Detailed data that allows for debugging */ - GIT_TRACE_DEBUG = 5, + GIT_TRACE_DEBUG = 0x0010u, /** Exceptionally detailed debugging data */ - GIT_TRACE_TRACE = 6 + GIT_TRACE_TRACE = 0x0020u, + + /** Performance tracking related traces */ + GIT_TRACE_PERF = 0x0040u, } git_trace_level_t; /** * An instance for a tracing function */ -typedef void (*git_trace_callback)(git_trace_level_t level, const char *msg); +typedef void (*git_trace_callback)( + git_trace_level_t level, /* just one bit will be sent */ + void *cb_payload, + void *msg_payload, + const char *msg); /** * Sets the system tracing configuration to the specified level with the * specified callback. When system events occur at a level equal to, or * lower than, the given level they will be reported to the given callback. * - * @param level Level to set tracing to - * @param cb Function to call with trace data + * @param level Bitmask of all enabled trace levels + * @param cb Function to call with trace messages + * @param cb_payload Payload to pass when callback is invoked * @return 0 or an error code */ -GIT_EXTERN(int) git_trace_set(git_trace_level_t level, git_trace_callback cb); +GIT_EXTERN(int) git_trace_set( + git_trace_level_t level, + git_trace_callback cb, + void *cb_payload); /** @} */ GIT_END_DECL diff --git a/src/diff.c b/src/diff.c index 8b7433c62..5a6b127a1 100644 --- a/src/diff.c +++ b/src/diff.c @@ -555,7 +555,7 @@ int git_diff__oid_for_entry( if (!entry.mode) { struct stat st; - git_trace(GIT_TRACE_TRACE, "stat=1"); + git_trace(GIT_TRACE_PERF, NULL, "stat"); if (p_stat(full_path.ptr, &st) < 0) { error = git_path_set_error(errno, entry.path, "stat"); git_buf_free(&full_path); @@ -570,7 +570,7 @@ int git_diff__oid_for_entry( if (S_ISGITLINK(entry.mode)) { git_submodule *sm; - git_trace(GIT_TRACE_TRACE, "submodule_lookup=1"); + git_trace(GIT_TRACE_PERF, NULL, "submodule_lookup"); if (!git_submodule_lookup(&sm, diff->repo, entry.path)) { const git_oid *sm_oid = git_submodule_wd_id(sm); if (sm_oid) @@ -583,7 +583,7 @@ int git_diff__oid_for_entry( giterr_clear(); } } else if (S_ISLNK(entry.mode)) { - git_trace(GIT_TRACE_TRACE, "oid_calculation=1"); + git_trace(GIT_TRACE_PERF, NULL, "oid_calculation"); 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'", @@ -596,7 +596,7 @@ int git_diff__oid_for_entry( if (fd < 0) error = fd; else { - git_trace(GIT_TRACE_TRACE, "oid_calculation=1"); + git_trace(GIT_TRACE_PERF, NULL, "oid_calculation"); error = git_odb__hashfd_filtered( out, fd, (size_t)entry.file_size, GIT_OBJ_BLOB, fl); p_close(fd); @@ -655,7 +655,7 @@ static int maybe_modified_submodule( ign == GIT_SUBMODULE_IGNORE_ALL) return 0; - git_trace(GIT_TRACE_TRACE, "submodule_lookup=1"); + git_trace(GIT_TRACE_PERF, NULL, "submodule_lookup"); if ((error = git_submodule_lookup( &sub, diff->repo, info->nitem->path)) < 0) { @@ -965,7 +965,7 @@ static int handle_unmatched_new_item( delta_type = GIT_DELTA_ADDED; else if (nitem->mode == GIT_FILEMODE_COMMIT) { - git_trace(GIT_TRACE_TRACE, "submodule_lookup=1"); + git_trace(GIT_TRACE_PERF, NULL, "submodule_lookup"); /* ignore things that are not actual submodules */ if (git_submodule_lookup(NULL, info->repo, nitem->path) != 0) { diff --git a/src/iterator.c b/src/iterator.c index bebdeba84..0d7e5918d 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -1018,7 +1018,7 @@ static int fs_iterator__expand_dir(fs_iterator *fi) return GIT_ENOTFOUND; } - git_trace(GIT_TRACE_TRACE, "stat=%ld", (long)ff->entries.length); + git_trace(GIT_TRACE_PERF, &ff->entries.length, "stat"); fs_iterator__seek_frame_start(fi, ff); @@ -1310,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_trace(GIT_TRACE_TRACE, "submodule_lookup=1"); + git_trace(GIT_TRACE_PERF, entry->path, "submodule_lookup"); if (git_submodule__is_submodule(fi->base.repo, entry->path)) { entry->st.st_mode = GIT_FILEMODE_COMMIT; entry->path_len--; diff --git a/src/trace.c b/src/trace.c index ee5039f56..6ee2cf2ce 100644 --- a/src/trace.c +++ b/src/trace.c @@ -17,22 +17,25 @@ struct git_trace_data git_trace__data = {0}; #endif -int git_trace_set(git_trace_level_t level, git_trace_callback callback) +int git_trace_set( + git_trace_level_t level, git_trace_callback cb, void *cb_payload) { #ifdef GIT_TRACE - assert(level == 0 || callback != NULL); + assert(level == 0 || cb != NULL); git_trace__data.level = level; - git_trace__data.callback = callback; + git_trace__data.callback = cb; + git_trace__data.callback_payload = cb_payload; GIT_MEMORY_BARRIER; return 0; #else GIT_UNUSED(level); - GIT_UNUSED(callback); + GIT_UNUSED(cb); + GIT_UNUSED(cb_payload); - giterr_set(GITERR_INVALID, - "This version of libgit2 was not built with tracing."); + giterr_set( + GITERR_INVALID, "This version of libgit2 was not built with tracing."); return -1; #endif } diff --git a/src/trace.h b/src/trace.h index 4d4e3bf53..b35e3808f 100644 --- a/src/trace.h +++ b/src/trace.h @@ -15,13 +15,16 @@ struct git_trace_data { git_trace_level_t level; git_trace_callback callback; + void *callback_payload; }; extern struct git_trace_data git_trace__data; GIT_INLINE(void) git_trace__write_fmt( git_trace_level_t level, - const char *fmt, ...) + void *message_payload, + const char *fmt, + ...) { git_trace_callback callback = git_trace__data.callback; git_buf message = GIT_BUF_INIT; @@ -31,18 +34,18 @@ GIT_INLINE(void) git_trace__write_fmt( git_buf_vprintf(&message, fmt, ap); va_end(ap); - callback(level, git_buf_cstr(&message)); + callback( + level, git_trace__data.callback_payload, message_payload, + git_buf_cstr(&message)); git_buf_free(&message); } #define git_trace_level() (git_trace__data.level) -#define git_trace(l, ...) { \ - if (git_trace__data.level >= l && \ - git_trace__data.callback != NULL) { \ - git_trace__write_fmt(l, __VA_ARGS__); \ - } \ - } +#define git_trace(l, p, ...) do { \ + if ((git_trace__data.level & (l)) != 0 && git_trace__data.callback) { \ + git_trace__write_fmt((l), (p), __VA_ARGS__); \ + } } while (0) #else diff --git a/tests/diff/diff_helpers.c b/tests/diff/diff_helpers.c index 279cb20c5..5de9834ba 100644 --- a/tests/diff/diff_helpers.c +++ b/tests/diff/diff_helpers.c @@ -229,3 +229,22 @@ void diff_print_raw(FILE *fp, git_diff *diff) git_diff_print(diff, GIT_DIFF_FORMAT_RAW, git_diff_print_callback__to_file_handle, fp ? fp : stderr)); } + +void diff_perf_track_stats( + git_trace_level_t level, + void *cb_payload, + void *msg_payload, + const char *msg) +{ + diff_perf *data = cb_payload; + + if (!(level & GIT_TRACE_PERF)) + return; + + if (!strcmp("stat", msg)) + data->stat_calls += msg_payload ? *((size_t *)msg_payload) : 1; + else if (!strcmp("submodule_lookup", msg)) + data->submodule_lookups++; + else if (!strcmp("oid_calculation", msg)) + data->oid_calcs++; +} diff --git a/tests/diff/diff_helpers.h b/tests/diff/diff_helpers.h index bf21f4b1f..3ed538702 100644 --- a/tests/diff/diff_helpers.h +++ b/tests/diff/diff_helpers.h @@ -62,3 +62,17 @@ extern int diff_foreach_via_iterator( extern void diff_print(FILE *fp, git_diff *diff); extern void diff_print_raw(FILE *fp, git_diff *diff); + +#include "git2/trace.h" + +typedef struct { + size_t stat_calls; + size_t oid_calcs; + size_t submodule_lookups; +} diff_perf; + +extern void diff_perf_track_stats( + git_trace_level_t level, + void *cb_payload, + void *msg_payload, + const char *msg); diff --git a/tests/diff/workdir.c b/tests/diff/workdir.c index 0fd41d3e0..952c9022b 100644 --- a/tests/diff/workdir.c +++ b/tests/diff/workdir.c @@ -1,40 +1,19 @@ #include "clar_libgit2.h" #include "diff_helpers.h" #include "repository.h" -#include static git_repository *g_repo = NULL; #ifdef GIT_TRACE -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); -} +static diff_perf g_diff_perf; #endif void test_diff_workdir__initialize(void) { #ifdef GIT_TRACE memset(&g_diff_perf, 0, sizeof(g_diff_perf)); - cl_git_pass(git_trace_set(GIT_TRACE_TRACE, add_stats)); + cl_git_pass(git_trace_set( + GIT_TRACE_PERF, diff_perf_track_stats, &g_diff_perf)); #endif } @@ -42,7 +21,7 @@ void test_diff_workdir__cleanup(void) { cl_git_sandbox_cleanup(); #ifdef GIT_TRACE - cl_git_pass(git_trace_set(0, NULL)); + cl_git_pass(git_trace_set(GIT_TRACE_NONE, NULL, NULL)); #endif } diff --git a/tests/status/worktree.c b/tests/status/worktree.c index c1d6be982..06864ad59 100644 --- a/tests/status/worktree.c +++ b/tests/status/worktree.c @@ -5,38 +5,18 @@ #include "posix.h" #include "util.h" #include "path.h" -#include +#include "../diff/diff_helpers.h" #ifdef GIT_TRACE -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); -} +static diff_perf g_diff_perf; #endif void test_status_worktree__initialize(void) { #ifdef GIT_TRACE memset(&g_diff_perf, 0, sizeof(g_diff_perf)); - cl_git_pass(git_trace_set(GIT_TRACE_TRACE, add_stats)); + cl_git_pass(git_trace_set( + GIT_TRACE_PERF, diff_perf_track_stats, &g_diff_perf)); #endif } @@ -50,7 +30,7 @@ void test_status_worktree__cleanup(void) { cl_git_sandbox_cleanup(); #ifdef GIT_TRACE - cl_git_pass(git_trace_set(0, NULL)); + cl_git_pass(git_trace_set(GIT_TRACE_NONE, NULL, NULL)); #endif } @@ -956,7 +936,5 @@ void test_status_worktree__update_stat_cache_0(void) 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 } diff --git a/tests/trace/trace.c b/tests/trace/trace.c index 87b325378..328539379 100644 --- a/tests/trace/trace.c +++ b/tests/trace/trace.c @@ -3,44 +3,49 @@ static int written = 0; -static void trace_callback(git_trace_level_t level, const char *message) +static void trace_callback( + git_trace_level_t level, + void *cb_payload, + void *msg_payload, + const char *msg) { - GIT_UNUSED(level); + GIT_UNUSED(level); GIT_UNUSED(msg_payload); - cl_assert(strcmp(message, "Hello world!") == 0); + cl_assert(strcmp(msg, "Hello world!") == 0); - written = 1; + if (cb_payload) + *((int *)cb_payload) = 1; } void test_trace_trace__initialize(void) { - git_trace_set(GIT_TRACE_INFO, trace_callback); + git_trace_set(GIT_TRACE_INFO_AND_BELOW, trace_callback, &written); written = 0; } void test_trace_trace__cleanup(void) { - git_trace_set(GIT_TRACE_NONE, NULL); + git_trace_set(GIT_TRACE_NONE, NULL, NULL); } void test_trace_trace__sets(void) { #ifdef GIT_TRACE - cl_assert(git_trace_level() == GIT_TRACE_INFO); + cl_assert(git_trace_level() == GIT_TRACE_INFO_AND_BELOW); #endif } void test_trace_trace__can_reset(void) { #ifdef GIT_TRACE - cl_assert(git_trace_level() == GIT_TRACE_INFO); - cl_git_pass(git_trace_set(GIT_TRACE_ERROR, trace_callback)); + cl_assert(git_trace_level() == GIT_TRACE_INFO_AND_BELOW); + cl_git_pass(git_trace_set(GIT_TRACE_ERROR, trace_callback, &written)); cl_assert(written == 0); - git_trace(GIT_TRACE_INFO, "Hello %s!", "world"); + git_trace(GIT_TRACE_INFO, NULL, "Hello %s!", "world"); cl_assert(written == 0); - git_trace(GIT_TRACE_ERROR, "Hello %s!", "world"); + git_trace(GIT_TRACE_ERROR, NULL, "Hello %s!", "world"); cl_assert(written == 1); #endif } @@ -48,13 +53,13 @@ void test_trace_trace__can_reset(void) void test_trace_trace__can_unset(void) { #ifdef GIT_TRACE - cl_assert(git_trace_level() == GIT_TRACE_INFO); - cl_git_pass(git_trace_set(GIT_TRACE_NONE, NULL)); + cl_assert(git_trace_level() == GIT_TRACE_INFO_AND_BELOW); + cl_git_pass(git_trace_set(GIT_TRACE_NONE, NULL, NULL)); cl_assert(git_trace_level() == GIT_TRACE_NONE); cl_assert(written == 0); - git_trace(GIT_TRACE_FATAL, "Hello %s!", "world"); + git_trace(GIT_TRACE_FATAL, NULL, "Hello %s!", "world"); cl_assert(written == 0); #endif } @@ -63,7 +68,7 @@ void test_trace_trace__skips_higher_level(void) { #ifdef GIT_TRACE cl_assert(written == 0); - git_trace(GIT_TRACE_DEBUG, "Hello %s!", "world"); + git_trace(GIT_TRACE_DEBUG, NULL, "Hello %s!", "world"); cl_assert(written == 0); #endif } @@ -72,7 +77,7 @@ void test_trace_trace__writes(void) { #ifdef GIT_TRACE cl_assert(written == 0); - git_trace(GIT_TRACE_INFO, "Hello %s!", "world"); + git_trace(GIT_TRACE_INFO, NULL, "Hello %s!", "world"); cl_assert(written == 1); #endif } @@ -81,7 +86,7 @@ void test_trace_trace__writes_lower_level(void) { #ifdef GIT_TRACE cl_assert(written == 0); - git_trace(GIT_TRACE_ERROR, "Hello %s!", "world"); + git_trace(GIT_TRACE_ERROR, NULL, "Hello %s!", "world"); cl_assert(written == 1); #endif } -- cgit v1.2.3