mbox series

[v4,0/8] Trace2 timers and counters and some cleanup

Message ID pull.1373.v4.git.1666618868.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Trace2 timers and counters and some cleanup | expand

Message

Philippe Blain via GitGitGadget Oct. 24, 2022, 1:40 p.m. UTC
Here is version 4 of this series to add timers and counters to Trace2.

Changes since V3:

 * Fixed typo in the new thread-name documentation.
 * Use a simpler NS_TO_SEC() macro for reporting the timer values.

Jeff Hostetler (8):
  trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx
  tr2tls: clarify TLS terminology
  api-trace2.txt: elminate section describing the public trace2 API
  trace2: rename the thread_name argument to trace2_thread_start
  trace2: improve thread-name documentation in the thread-context
  trace2: convert ctx.thread_name from strbuf to pointer
  trace2: add stopwatch timers
  trace2: add global counter mechanism

 Documentation/technical/api-trace2.txt | 190 +++++++++++++++++--------
 Makefile                               |   2 +
 t/helper/test-trace2.c                 | 187 ++++++++++++++++++++++++
 t/t0211-trace2-perf.sh                 |  95 +++++++++++++
 t/t0211/scrub_perf.perl                |   6 +
 trace2.c                               | 121 +++++++++++++++-
 trace2.h                               | 101 +++++++++++--
 trace2/tr2_ctr.c                       | 101 +++++++++++++
 trace2/tr2_ctr.h                       | 104 ++++++++++++++
 trace2/tr2_tgt.h                       |  16 +++
 trace2/tr2_tgt_event.c                 |  47 +++++-
 trace2/tr2_tgt_normal.c                |  39 +++++
 trace2/tr2_tgt_perf.c                  |  43 +++++-
 trace2/tr2_tls.c                       |  34 +++--
 trace2/tr2_tls.h                       |  55 ++++---
 trace2/tr2_tmr.c                       | 182 +++++++++++++++++++++++
 trace2/tr2_tmr.h                       | 140 ++++++++++++++++++
 17 files changed, 1361 insertions(+), 102 deletions(-)
 create mode 100644 trace2/tr2_ctr.c
 create mode 100644 trace2/tr2_ctr.h
 create mode 100644 trace2/tr2_tmr.c
 create mode 100644 trace2/tr2_tmr.h


base-commit: 3dcec76d9df911ed8321007b1d197c1a206dc164
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1373%2Fjeffhostetler%2Ftrace2-stopwatch-v4-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1373/jeffhostetler/trace2-stopwatch-v4-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1373

Range-diff vs v3:

 1:  6e7e4f3187e = 1:  6e7e4f3187e trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx
 2:  9dee7a75903 = 2:  9dee7a75903 tr2tls: clarify TLS terminology
 3:  804dab9e1a7 = 3:  804dab9e1a7 api-trace2.txt: elminate section describing the public trace2 API
 4:  9adf9cee1a9 = 4:  9adf9cee1a9 trace2: rename the thread_name argument to trace2_thread_start
 5:  8cb206b7632 ! 5:  acfae17548c trace2: improve thread-name documentation in the thread-context
     @@ trace2/tr2_tls.h: struct tr2tls_thread_ctx {
      + * Subsequent threads are given a non-zero thread_id and a thread_name
      + * constructed from the id and a thread base name (which is usually just
      + * the name of the thread-proc function).  For example:
     -+ *     { .thread_id=10, .thread_name="th10fsm-listen" }
     ++ *     { .thread_id=10, .thread_name="th10:fsm-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.
 6:  8a89e1aa238 = 6:  79c6406d492 trace2: convert ctx.thread_name from strbuf to pointer
 7:  8e701109976 ! 7:  a10c1bd96bb trace2: add stopwatch timers
     @@ trace2/tr2_tgt.h
      +struct tr2_timer_metadata;
      +struct tr2_timer;
      +
     -+#define NS_PER_SEC_D ((double)1000*1000*1000)
     ++#define NS_TO_SEC(ns) ((double)(ns) / 1.0e9)
       
       /*
        * Function prototypes for a TRACE2 "target" vtable.
     @@ trace2/tr2_tgt_event.c: static void fn_data_json_fl(const char *file, int line,
      +{
      +	const char *event_name = is_final_data ? "timer" : "th_timer";
      +	struct json_writer jw = JSON_WRITER_INIT;
     -+	double t_total = ((double)timer->total_ns) / NS_PER_SEC_D;
     -+	double t_min = ((double)timer->min_ns) / NS_PER_SEC_D;
     -+	double t_max = ((double)timer->max_ns) / NS_PER_SEC_D;
     ++	double t_total = NS_TO_SEC(timer->total_ns);
     ++	double t_min = NS_TO_SEC(timer->min_ns);
     ++	double t_max = NS_TO_SEC(timer->max_ns);
      +
      +	jw_object_begin(&jw, 0);
      +	event_fmt_prepare(event_name, __FILE__, __LINE__, NULL, &jw);
     @@ trace2/tr2_tgt_normal.c: static void fn_printf_va_fl(const char *file, int line,
      +{
      +	const char *event_name = is_final_data ? "timer" : "th_timer";
      +	struct strbuf buf_payload = STRBUF_INIT;
     -+	double t_total = ((double)timer->total_ns) / NS_PER_SEC_D;
     -+	double t_min = ((double)timer->min_ns) / NS_PER_SEC_D;
     -+	double t_max = ((double)timer->max_ns) / NS_PER_SEC_D;
     ++	double t_total = NS_TO_SEC(timer->total_ns);
     ++	double t_min = NS_TO_SEC(timer->min_ns);
     ++	double t_max = NS_TO_SEC(timer->max_ns);
      +
      +	strbuf_addf(&buf_payload, ("%s %s/%s"
      +				   " intervals:%"PRIu64
     @@ trace2/tr2_tgt_perf.c: static void fn_printf_va_fl(const char *file, int line,
      +{
      +	const char *event_name = is_final_data ? "timer" : "th_timer";
      +	struct strbuf buf_payload = STRBUF_INIT;
     -+	double t_total = ((double)timer->total_ns) / NS_PER_SEC_D;
     -+	double t_min = ((double)timer->min_ns) / NS_PER_SEC_D;
     -+	double t_max = ((double)timer->max_ns) / NS_PER_SEC_D;
     ++	double t_total = NS_TO_SEC(timer->total_ns);
     ++	double t_min = NS_TO_SEC(timer->min_ns);
     ++	double t_max = NS_TO_SEC(timer->max_ns);
      +
      +	strbuf_addf(&buf_payload, ("name:%s"
      +				   " intervals:%"PRIu64
 8:  5cd8bdde884 ! 8:  b359a49cec9 trace2: add global counter mechanism
     @@ trace2/tr2_tgt.h: struct repository;
      +struct tr2_counter_metadata;
      +struct tr2_counter;
       
     - #define NS_PER_SEC_D ((double)1000*1000*1000)
     + #define NS_TO_SEC(ns) ((double)(ns) / 1.0e9)
       
      @@ trace2/tr2_tgt.h: typedef void(tr2_tgt_evt_timer_t)(const struct tr2_timer_metadata *meta,
       				  const struct tr2_timer *timer,

Comments

Derrick Stolee Oct. 25, 2022, 12:27 p.m. UTC | #1
On 10/24/2022 9:40 AM, Jeff Hostetler via GitGitGadget wrote:
> Here is version 4 of this series to add timers and counters to Trace2.
> 
> Changes since V3:
> 
>  * Fixed typo in the new thread-name documentation.
>  * Use a simpler NS_TO_SEC() macro for reporting the timer values.
> 
> Jeff Hostetler (8):
>   trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx
>   tr2tls: clarify TLS terminology
>   api-trace2.txt: elminate section describing the public trace2 API
>   trace2: rename the thread_name argument to trace2_thread_start
>   trace2: improve thread-name documentation in the thread-context
>   trace2: convert ctx.thread_name from strbuf to pointer
>   trace2: add stopwatch timers
>   trace2: add global counter mechanism

I re-read the series as well as looked at the range-diffs for the
previous two versions. I continue to think this is a high-quality
series and I've used it multiple times in my personal development
workflow to investigate certain performance things. I'm looking
forward to this being merged so we can all use it.

Thanks,
-Stolee
Junio C Hamano Oct. 25, 2022, 3:36 p.m. UTC | #2
Derrick Stolee <derrickstolee@github.com> writes:

> On 10/24/2022 9:40 AM, Jeff Hostetler via GitGitGadget wrote:
>> Here is version 4 of this series to add timers and counters to Trace2.
>> 
>> Changes since V3:
>> 
>>  * Fixed typo in the new thread-name documentation.
>>  * Use a simpler NS_TO_SEC() macro for reporting the timer values.
>> 
>> Jeff Hostetler (8):
>>   trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx
>>   tr2tls: clarify TLS terminology
>>   api-trace2.txt: elminate section describing the public trace2 API
>>   trace2: rename the thread_name argument to trace2_thread_start
>>   trace2: improve thread-name documentation in the thread-context
>>   trace2: convert ctx.thread_name from strbuf to pointer
>>   trace2: add stopwatch timers
>>   trace2: add global counter mechanism
>
> I re-read the series as well as looked at the range-diffs for the
> previous two versions. I continue to think this is a high-quality
> series and I've used it multiple times in my personal development
> workflow to investigate certain performance things. I'm looking
> forward to this being merged so we can all use it.

I agree with your assessment.  Let's move it forward.

Thanks, all.