Re: [PATCH] libtraceeval: Add traceeval_stat_max/min_timestamp() API

[Ross Zwisler <zwisler@google.com>]

On Wed, Oct 11, 2023 at 07:09:17PM -0400, Steven Rostedt wrote:
> On Wed, 11 Oct 2023 16:21:00 -0600
> Ross Zwisler <zwisler@google.com> wrote:
> 
> > On Thu, Oct 05, 2023 at 06:14:48PM -0400, Steven Rostedt wrote:
> > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > > 
> > > If a value field is flagged as a TIMESTAMP, and another field is flagged
> > > as a STAT, have the statistics for the STAT field automatically keep track
> > > of when the max and min happened (what the TIMESTAMP field was at those
> > > instances).  
> > 
> > I'm confused as to why we have TRACEEVAL_FL_TIMESTAMP and a separate data
> > entry with that flag, instead of only storing the timestamp in the entry
> > metadata, as we do with the other STATs?
> 
> Actually, I may remove this part entirely, as it's pretty much superseded
> by the TRACEEVAL_TYPE_DELTA, which I moved to, and haven't needed the
> separate val marked for the timestamp.
> 
> > 
> > >From the description above where we have one TIMESTAMP and one STAT, I would  
> > expect to see structures defined like this
> > (from [PATCH v2] libtraceeval: Add wake-lat sample code):
> > 
> > +struct traceeval_type sched_vals[] = {
> > +       {
> > +               .name           = "timestamp",
> > +               .flags          = TRACEEVAL_FL_TIMESTAMP,
> > +               .type           = TRACEEVAL_TYPE_NUMBER_64,
> > +       },
> > +       {
> > +               .name           = "delta",
> > +               .flags          = TRACEEVAL_FL_STAT,
> > +               .type           = TRACEEVAL_TYPE_NUMBER_64,
> > +       }
> > 
> > where the timestamp is sync'd with the STAT min and max, right?
> 
> The new type is done like:
> 
> 	{
> 		.name		= "delta",
> 		.type		= TRACEEVAL_TYPE_DELTA,
> 	}
> 
> Which will automatically be marked as STAT flag, and is set with:
> 
> 	TRACEEVAL_SET_DELTA(vals, delta, timestamp);
> 
> Is that what you are thinking about?

Yep, I think so.  I'll take a harder look at TRACEEVAL_TYPE_DELTA.