Re: [PATCH 2/9] libtraceeval: Add traceeval_iterator_query()

[Ross Zwisler <zwisler@google.com>]

On Thu, Aug 17, 2023 at 06:24:15PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> Add a query to the iterator that will return the values of the last entry
> that matches the keys returned by the last call of traceeval_iterator_next().
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/traceeval-hist.h |  2 ++
>  samples/task-eval.c      |  2 +-
>  src/histograms.c         | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> index 0cee9bcbeb83..837a74f61a66 100644
> --- a/include/traceeval-hist.h
> +++ b/include/traceeval-hist.h
> @@ -186,5 +186,7 @@ int traceeval_iterator_sort_custom(struct traceeval_iterator *iter,
>  				   traceeval_cmp_fn sort_fn, void *data);
>  int traceeval_iterator_next(struct traceeval_iterator *iter,
>  			    const union traceeval_data **keys);
> +int traceeval_iterator_query(struct traceeval_iterator *iter,
> +			     const union traceeval_data **results);
>  
>  #endif /* __LIBTRACEEVAL_HIST_H__ */
> diff --git a/samples/task-eval.c b/samples/task-eval.c
> index 66d0c40dc0c8..18e07f44e11e 100644
> --- a/samples/task-eval.c
> +++ b/samples/task-eval.c
> @@ -836,7 +836,7 @@ static void display_processes(struct traceeval *teval,
>  		struct process_data *pdata = NULL;
>  		const char *comm = keys[0].cstring;
>  
> -		ret = traceeval_query(teval_data, keys, &results);
> +		ret = traceeval_iterator_query(iter, &results);
>  		if (ret < 0)
>  			pdie("Could not query iterator");
>  		if (ret < 1)
> diff --git a/src/histograms.c b/src/histograms.c
> index 66bdc1769985..4f603ce36c8c 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
> @@ -1306,3 +1306,33 @@ int traceeval_iterator_next(struct traceeval_iterator *iter,
>  	*keys = entry->keys;
>  	return 1;
>  }
> +
> +/**
> + * traceeval_iterator_query - return the results of the last entry in the iterator
> + * @iter: The iterator to retrieve the entry results from
> + * @results: The returned results of the last entry (if exists)
> + *
> + * This returns the @results of the values from the last instance of
> + * traceeval_iterator_next(). It is equivalent of calling:
> + *
> + * traceeval_query_size() with the keys returned by traceeval_iterator_next().

      traceeval_query()

> + *
> + * Except that it will always quickly return the last entry, whereas the
> + * traceeval_query_size() will reset the cached next_entry and do a full

      traceeval_query()

Aside from these you can add:

Reviewed-by: Ross Zwisler <zwisler@google.com>

I do agree that having a dedicated traceeval_iterator_results_release() would
be nice.

> + * lookup again.
> + *
> + * Returns 1 if another entry is returned, or 0 if not (or negative on error)
> + */
> +int traceeval_iterator_query(struct traceeval_iterator *iter,
> +			     const union traceeval_data **results)
> +{
> +	struct entry *entry;
> +
> +	if (iter->next < 1 || iter->next > iter->nr_entries)
> +		return 0;
> +
> +	entry = iter->entries[iter->next - 1];
> +	*results = entry->vals;
> +
> +	return 1;
> +}
> -- 
> 2.40.1
>