On Tue, 15 Aug 2023 14:25:54 -0600
Ross Zwisler <zwisler@xxxxxxxxxx> wrote:
> 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.
Because the stats are for each key, not the total of the traceeval. If you
look at the code in task-eval, I want to know the max,min,total,count,avg
of the sleep time for a task when it is blocked, when it is sleeping, when
it is running, etc:
Task: migrate
Total run time (us): 4645364
Total blocked time (us): 0
Total preempt time (us): 98540
Total sleep time (us): 115559
thread id: 1884
Total run time (us): 808
thread id: 1885
Total run time (us): 2920763
Total preempt time (us): 245570
Total sleep time (us): 224289277
thread id: 1886
Total run time (us): 3601375
Total preempt time (us): 1308185
Total sleep time (us): 12692630
thread id: 1887
Total run time (us): 3740300
Total preempt time (us): 2620102
Total sleep time (us): 4806722
That "Task: migrate" is just one entry in the traceeval, the threads are
entries in the nested traceeval that is saved per task.
Actually, task-eval is only looking at the total time, but I could easily
add the max/min/avg times too. If I were to write a latency program, and
was tracing the wake up latency of a task, you probably want these stats
for each key, so I can see the max latency per task, and not just the total
for all tasks.
-- Steve
|