On Fri, Aug 11, 2023 at 01:39:33AM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> Whenever an entry is added that already exists (overwriting the values)
> keep track of the stats for the number values (max, min, total, count).
>
> Also move the stat structure out of the public view. We may want to modify
> this structure in the future, and so it should not become an API.
>
> Add accessor functions to get to the stat values.
>
> Add traceeval_stat() to acquire a stat handle from a specific key for a
> specific value.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> include/traceeval-hist.h | 17 ++--
> src/eval-local.h | 9 ++
> src/histograms.c | 182 +++++++++++++++++++++++++++++++++++----
> 3 files changed, 183 insertions(+), 25 deletions(-)
>
> diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> index 1c02f3039809..d061a4532b06 100644
> --- a/include/traceeval-hist.h
> +++ b/include/traceeval-hist.h
> @@ -130,13 +130,7 @@ struct traceeval_type {
> };
>
> /* Statistics about a given entry element */
> -struct traceeval_stat {
> - unsigned long long max;
> - unsigned long long min;
> - unsigned long long total;
> - unsigned long long avg;
> - unsigned long long std;
> -};
> +struct traceeval_stat;
>
> /* Iterator over aggregated data */
> struct traceeval_iterator;
> @@ -160,4 +154,13 @@ int traceeval_query(struct traceeval *teval, const union traceeval_data *keys,
> void traceeval_results_release(struct traceeval *teval,
> union traceeval_data *results);
>
> +struct traceeval_stat *traceeval_stat(struct traceeval *teval,
> + const union traceeval_data *keys,
> + struct traceeval_type *type);
> +
> +unsigned long long traceeval_stat_max(struct traceeval_stat *stat);
> +unsigned long long traceeval_stat_min(struct traceeval_stat *stat);
> +unsigned long long traceeval_stat_total(struct traceeval_stat *stat);
> +unsigned long long traceeval_stat_count(struct traceeval_stat *stat);
> +
> #endif /* __LIBTRACEEVAL_HIST_H__ */
> diff --git a/src/eval-local.h b/src/eval-local.h
> index 820d7ad096e8..190b19db14d2 100644
> --- a/src/eval-local.h
> +++ b/src/eval-local.h
> @@ -45,11 +45,20 @@ struct hash_table {
> struct hash_iter iter;
> };
>
> +struct traceeval_stat {
> + unsigned long long max;
> + unsigned long long min;
> + unsigned long long total;
> + unsigned long long std;
> + size_t count;
> +};
> +
> /* A key-value pair */
> struct entry {
> struct hash_item hash;
> union traceeval_data *keys;
> union traceeval_data *vals;
> + struct traceeval_stat *val_stats;
I'm confused about why 'val_stats' is in 'struct entry', instead of in
'struct traceeval'?
I would think that we'd want to keep our stats on a per-histogram basis, where
we have an array of 'struct traceeval_stat' structs with the same size as our
'val_types' array, so that for a given compound value, say
{ TRACEVAL_TYPE_NUMBER_64, TRACEEVAL_TYPE_NUMBER_16, TRACEEVAL_NUMBER_8}
we'd have stats for each of these entries, like min, max, average, etc.?
With the stats associated with each entry, I don't see how we keep all the
stats for all the entries up to date as we do insertions & removals.
|