On Fri, Aug 11, 2023 at 01:39:37AM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx> > > 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@xxxxxxxxxxx> > --- > src/histograms.c | 48 ++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 34 insertions(+), 14 deletions(-) > <> > @@ -772,20 +778,34 @@ fail_entry: > * > * Frees the old vals field of @entry, unless an error occurs. > * > - * Return 0 on success, -1 on error. > + * Return 1 on success, -1 on error. The code is now returning 1, 0 and -1 in different cases, and all three of these return values are percolating up to be returned by traceeval_insert(), which is only supposed to return 0 or 1. > */ > 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 1; If we try and copy a 0 length val, is it okay to just return 0 here and call it success? > + > + for (i = 0; i < size; i++) { > + old[i] = copy[i]; > + > + if (copy_traceeval_data(types + i, stats + i, > + vals + i, copy + i)) > + goto fail; > + } I think we still need to rip through old[] and free strings, and also call .release on types that define it, probably via data_release(). I don't understand why data_release() only calls .release if a .copy function isn't defined? Are we saying that .copy() must also release the old data? If so that needs to be explicit, but I think we still need to free strings at a minimum. > > - clean_data_set(entry->vals, teval->val_types, teval->nr_val_types); > - entry->vals = new_vals; > return 0; > + > +fail: > + data_release(i, old, types); > + return -1; > } > > struct traceeval_stat *traceeval_stat(struct traceeval *teval, > -- > 2.40.1 >
![]() |