From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
Rename the `thread_name` argument in `tr2tls_create_self()`
and `trace2_thread_start()` to be `name_hint` to make it clear
that the passed argument is a hint that will be used to create
the actual `struct tr2tls_thread_ctx.thread_name` variable.
This should make it clearer in the API that the trace2 layer
does not borrow the caller's string pointer/buffer, but rather
that it will use that hint in formatting the actual thread's
name. Previous discussion on the mailing list indicated that
there was confusion about this point.
This commit does not change how the `thread_name` field is
allocated or stored within the `tr2tls_thread_ctx` structure.
Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
---
Documentation/technical/api-trace2.txt | 2 +-
trace2.c | 6 +++---
trace2.h | 11 ++++++-----
trace2/tr2_tls.c | 4 ++--
trace2/tr2_tls.h | 17 ++++++++++-------
5 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
index 431d424f9d5..4fe2d6992ab 100644
--- a/Documentation/technical/api-trace2.txt
+++ b/Documentation/technical/api-trace2.txt
@@ -209,7 +209,7 @@ e.g: `void trace2_child_start(struct child_process *cmd)`.
These messages are concerned with Git thread usage.
-e.g: `void trace2_thread_start(const char *thread_name)`.
+e.g: `void trace2_thread_start(const char *name_hint)`.
=== Region and Data Messages
diff --git a/trace2.c b/trace2.c
index c1244e45ace..c8e5acced2a 100644
--- a/trace2.c
+++ b/trace2.c
@@ -466,7 +466,7 @@ void trace2_exec_result_fl(const char *file, int line, int exec_id, int code)
file, line, us_elapsed_absolute, exec_id, code);
}
-void trace2_thread_start_fl(const char *file, int line, const char *thread_name)
+void trace2_thread_start_fl(const char *file, int line, const char *name_hint)
{
struct tr2_tgt *tgt_j;
int j;
@@ -488,14 +488,14 @@ void trace2_thread_start_fl(const char *file, int line, const char *thread_name)
*/
trace2_region_enter_printf_fl(file, line, NULL, NULL, NULL,
"thread-proc on main: %s",
- thread_name);
+ name_hint);
return;
}
us_now = getnanotime() / 1000;
us_elapsed_absolute = tr2tls_absolute_elapsed(us_now);
- tr2tls_create_self(thread_name, us_now);
+ tr2tls_create_self(name_hint, us_now);
for_each_wanted_builtin (j, tgt_j)
if (tgt_j->pfn_thread_start_fl)
diff --git a/trace2.h b/trace2.h
index af3c11694cc..fe39dcb5849 100644
--- a/trace2.h
+++ b/trace2.h
@@ -304,14 +304,15 @@ void trace2_exec_result_fl(const char *file, int line, int exec_id, int code);
* thread-proc to allow the thread to create its own thread-local
* storage.
*
- * Thread names should be descriptive, like "preload_index".
- * Thread names will be decorated with an instance number automatically.
+ * The thread name hint should be descriptive, like "preload_index" or
+ * taken from the thread-proc function. A unique thread name will be
+ * created from the hint and the thread id automatically.
*/
void trace2_thread_start_fl(const char *file, int line,
- const char *thread_name);
+ const char *name_hint);
-#define trace2_thread_start(thread_name) \
- trace2_thread_start_fl(__FILE__, __LINE__, (thread_name))
+#define trace2_thread_start(name_hint) \
+ trace2_thread_start_fl(__FILE__, __LINE__, (name_hint))
/*
* Emit a 'thread_exit' event. This must be called from inside the
diff --git a/trace2/tr2_tls.c b/trace2/tr2_tls.c
index 8d2182fbdbb..39b41fd2487 100644
--- a/trace2/tr2_tls.c
+++ b/trace2/tr2_tls.c
@@ -31,7 +31,7 @@ void tr2tls_start_process_clock(void)
tr2tls_us_start_process = getnanotime() / 1000;
}
-struct tr2tls_thread_ctx *tr2tls_create_self(const char *thread_name,
+struct tr2tls_thread_ctx *tr2tls_create_self(const char *name_hint,
uint64_t us_thread_start)
{
struct tr2tls_thread_ctx *ctx = xcalloc(1, sizeof(*ctx));
@@ -50,7 +50,7 @@ struct tr2tls_thread_ctx *tr2tls_create_self(const char *thread_name,
strbuf_init(&ctx->thread_name, 0);
if (ctx->thread_id)
strbuf_addf(&ctx->thread_name, "th%02d:", ctx->thread_id);
- strbuf_addstr(&ctx->thread_name, thread_name);
+ strbuf_addstr(&ctx->thread_name, name_hint);
if (ctx->thread_name.len > TR2_MAX_THREAD_NAME)
strbuf_setlen(&ctx->thread_name, TR2_MAX_THREAD_NAME);
diff --git a/trace2/tr2_tls.h b/trace2/tr2_tls.h
index 1297509fd23..f1ee58305d6 100644
--- a/trace2/tr2_tls.h
+++ b/trace2/tr2_tls.h
@@ -25,17 +25,20 @@ struct tr2tls_thread_ctx {
/*
* Create thread-local storage for the current thread.
*
- * We assume the first thread is "main". Other threads are given
- * non-zero thread-ids to help distinguish messages from concurrent
- * threads.
- *
- * Truncate the thread name if necessary to help with column alignment
- * in printf-style messages.
+ * The first thread in the process will have:
+ * { .thread_id=0, .thread_name="main" }
+ * Subsequent threads are given a non-zero thread_id and a thread_name
+ * constructed from the id and a "name hint" (which is usually based
+ * upon the name of the thread-proc function). For example:
+ * { .thread_id=10, .thread_name="th10fsm-listen" }
+ * This helps to identify and distinguish messages from concurrent threads.
+ * The ctx.thread_name field is truncated if necessary to help with column
+ * alignment in printf-style messages.
*
* In this and all following functions the term "self" refers to the
* current thread.
*/
-struct tr2tls_thread_ctx *tr2tls_create_self(const char *thread_name,
+struct tr2tls_thread_ctx *tr2tls_create_self(const char *name_hint,
uint64_t us_thread_start);
/*
--
gitgitgadget
