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

@@ -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

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) {

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--;

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
 }

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
 

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++;
+}

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);

tests/diff/workdir.c

 #include "clar_libgit2.h"
 #include "diff_helpers.h"
 #include "repository.h"
-#include <git2/trace.h>
 
 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
 }
 

tests/status/worktree.c

@@ -5,38 +5,18 @@
 #include "posix.h"
 #include "util.h"
 #include "path.h"
-#include <git2/trace.h>
+#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
 }

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
 }