Re: [PATCH v3 14/18] libtraceeval histogram: Use stack for old copy in update

[Ross Zwisler <zwisler@google.com>]

On Wed, Aug 16, 2023 at 09:33:06PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> In the update, instead of using the heap, use the stack to save the old
> values. This should save time where it does not need to allocate and then
> free the value list to do an update.
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  src/histograms.c | 51 ++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 38 insertions(+), 13 deletions(-)
> 
> diff --git a/src/histograms.c b/src/histograms.c
> index 57b4b2166e3b..3050bad4190e 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
<>
> @@ -776,15 +782,34 @@ fail_entry:
>  static int update_entry(struct traceeval *teval, struct entry *entry,
>  			const union traceeval_data *vals)
>  {
> -	union traceeval_data *new_vals;
> +	struct traceeval_stat *stats = entry->val_stats;
> +	struct traceeval_type *types = teval->val_types;
> +	union traceeval_data *copy = entry->vals;
> +	union traceeval_data old[teval->nr_val_types];
> +	size_t size = teval->nr_val_types;
> +	size_t i;
>  
> -	if (copy_traceeval_data_set(teval->nr_val_types, teval->val_types,
> -				vals, entry->val_stats, &new_vals) == -1)
> -		return -1;
> +	if (!size)
> +		return 0;
>  
> -	clean_data_set(entry->vals, teval->val_types, teval->nr_val_types);
> -	entry->vals = new_vals;
> +	for (i = 0; i < size; i++) {
> +		old[i] = copy[i];
> +
> +		if (copy_traceeval_data(types + i, stats + i,
> +					vals + i, copy + i))
> +			goto fail;
> +	}
> +	data_release(size, old, types);
>  	return 0;
> + fail:
> +	/* Free the new values that were added */
> +	data_release(i, copy, types);
> +	/* Put back the old values */
> +	for (i--; i >= 0; i--) {
> +		copy_traceeval_data(types + i, NULL,
> +				    copy + i, old + i);

should be:                          old + i, copy + i);

Right now we're copying 'copy' into 'old', but we want to be doing it the
other way around so we restore 'copy' back to its saved state in 'old'

> +	}
> +	return -1;
>  }
>  
>  struct traceeval_stat *traceeval_stat(struct traceeval *teval,
> -- 
> 2.40.1
>