Re: [RFC PATCH 1/8] trace2: log fsync stats in trace2 rather than wrapper
- From
- Jeff Hostetler <git@jeffhostetler.com>
- Date
- Jul 11, 2023, 20:07 UTC
- Message-ID
- <7c8d6012-5bb5-9ee5-eda1-80e46c4b6981@jeffhostetler.com>
- In-Reply-To
- <20230627195251.1973421-2-calvinwan@google.com>
On 6/27/23 3:52 PM, Calvin Wan wrote:
Show 48 quoted lines
> As a library boundary, wrapper.c should not directly log trace2
> statistics, but instead provide those statistics upon
> request. Therefore, move the trace2 logging code to trace2.[ch.]. This
> also allows wrapper.c to not be dependent on trace2.h and repository.h.
>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> ---
> trace2.c | 13 +++++++++++++
> trace2.h | 5 +++++
> wrapper.c | 17 ++++++-----------
> wrapper.h | 4 ++--
> 4 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/trace2.c b/trace2.c
> index 0efc4e7b95..f367a1ce31 100644
> --- a/trace2.c
> +++ b/trace2.c
> @@ -915,3 +915,16 @@ const char *trace2_session_id(void)
> {
> return tr2_sid_get();
> }
> +
> +static void log_trace_fsync_if(const char *key)
> +{
> + intmax_t value = get_trace_git_fsync_stats(key);
> + if (value)
> + trace2_data_intmax("fsync", the_repository, key, value);
> +}
> +
> +void trace_git_fsync_stats(void)
> +{
> + log_trace_fsync_if("fsync/writeout-only");
> + log_trace_fsync_if("fsync/hardware-flush");
> +}
> diff --git a/trace2.h b/trace2.h
> index 4ced30c0db..689e9a4027 100644
> --- a/trace2.h
> +++ b/trace2.h
> @@ -581,4 +581,9 @@ void trace2_collect_process_info(enum trace2_process_info_reason reason);
>
> const char *trace2_session_id(void);
>
> +/*
> + * Writes out trace statistics for fsync
> + */
> +void trace_git_fsync_stats(void);
> +
> #endif /* TRACE2_H */Sorry to be late to this party, but none of this belongs in trace2.[ch].
As Victoria stated, you can/should use the new "timers and counters" feature in Trace2 to collect and log these stats.
And then you don't need specific log_trace_* functions or wrappers -- just use the trace2_timer_start()/stop() or trace2_counter_add() functions as necessary around the various fsync operations.
I haven't really followed the lib-ification effort, so I'm just going to GUESS that all of the Trace2_ and tr2_ prefixed functions and data structures will need to be in the lowest-level .a so that it can be called from the main .exe and any other .a's between them.
Jeff