| Message ID | 20230811053940.1408424-16-rostedt@goodmis.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libtraceeval histogram: Updates | expand |
On Fri, Aug 11, 2023 at 01:39:38AM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > Add an iterator where the application can supply the sort algorithm where > it gets the teval descriptor along with the keys and values of both of the > entries to compare against. Also, allow it to submit its own data to the > compare function: > > int traceeval_iterator_sort_custom(struct traceeval_iterator *iter, > traceeval_cmp_fn sort_fn, void *data); > > with > > typedef int (*traceeval_cmp_fn)(struct traceeval *teval, > const union traceeval_data *Akeys, > const union traceeval_data *Avals, > const union traceeval_data *Bkeys, > const union traceeval_data *Bvals, > void *data); > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > include/traceeval-hist.h | 9 +++++++++ > src/histograms.c | 34 ++++++++++++++++++++++++++++++++++ > 2 files changed, 43 insertions(+) > > diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h > index 1a24d6117b93..839f63630897 100644 > --- a/include/traceeval-hist.h > +++ b/include/traceeval-hist.h > @@ -86,6 +86,13 @@ typedef int (*traceeval_data_copy_fn)(const struct traceeval_type *type, > union traceeval_data *copy, > const union traceeval_data *origin); > > +typedef int (*traceeval_cmp_fn)(struct traceeval *teval, > + const union traceeval_data *Akeys, > + const union traceeval_data *Avals, > + const union traceeval_data *Bkeys, > + const union traceeval_data *Bvals, > + void *data); > + > /* > * struct traceeval_type - Describes the type of a traceevent_data instance > * @type: The enum type that describes the traceeval_data > @@ -172,6 +179,8 @@ struct traceeval_iterator *traceeval_iterator_get(struct traceeval *teval); > void traceeval_iterator_put(struct traceeval_iterator *iter); > int traceeval_iterator_sort(struct traceeval_iterator *iter, const char *sort_field, > int level, bool ascending); > +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); > > diff --git a/src/histograms.c b/src/histograms.c > index 643a550422f6..33c87644d468 100644 > --- a/src/histograms.c > +++ b/src/histograms.c > @@ -1153,6 +1153,40 @@ static int sort_iter(struct traceeval_iterator *iter) > return 0; > } > > +struct iter_custom_data { > + struct traceeval_iterator *iter; > + traceeval_cmp_fn sort_fn; > + void *data; > +}; > + > +static int iter_custom_cmp(const void *A, const void *B, void *data) > +{ > + struct iter_custom_data *cust_data = data; > + struct traceeval_iterator *iter = cust_data->iter; > + struct traceeval *teval = iter->teval; > + const struct entry *a = *((const struct entry **)A); > + const struct entry *b = *((const struct entry **)B); > + > + return cust_data->sort_fn(teval, a->keys, a->vals, b->keys, b->vals, > + cust_data->data); > +} > + > +int traceeval_iterator_sort_custom(struct traceeval_iterator *iter, > + traceeval_cmp_fn sort_fn, void *data) > +{ > + struct iter_custom_data cust_data = { > + .iter = iter, > + .sort_fn = sort_fn, > + .data = data > + }; > + > + qsort_r(iter->entries, iter->nr_entries, sizeof(*iter->entries), > + iter_custom_cmp, &cust_data); I guess I don't yet see what this gives us over the existing sorting and iterators? Does this do the same thing, we just pass in the sort function instead of calling traceeval_iterator_sort() one or more times? > + > + iter->needs_sort = false; Also probably need to set iter->next = 0; > + return 0; > +} > + > /** > * traceeval_iterator_next - retrieve the next entry from an iterator > * @iter: The iterator to retrieve the next entry from > -- > 2.40.1 >
On Wed, 16 Aug 2023 16:57:16 -0600 Ross Zwisler <zwisler@google.com> wrote: > > +static int iter_custom_cmp(const void *A, const void *B, void *data) > > +{ > > + struct iter_custom_data *cust_data = data; > > + struct traceeval_iterator *iter = cust_data->iter; > > + struct traceeval *teval = iter->teval; > > + const struct entry *a = *((const struct entry **)A); > > + const struct entry *b = *((const struct entry **)B); > > + > > + return cust_data->sort_fn(teval, a->keys, a->vals, b->keys, b->vals, > > + cust_data->data); > > +} > > + > > +int traceeval_iterator_sort_custom(struct traceeval_iterator *iter, > > + traceeval_cmp_fn sort_fn, void *data) > > +{ > > + struct iter_custom_data cust_data = { > > + .iter = iter, > > + .sort_fn = sort_fn, > > + .data = data > > + }; > > + > > + qsort_r(iter->entries, iter->nr_entries, sizeof(*iter->entries), > > + iter_custom_cmp, &cust_data); > > I guess I don't yet see what this gives us over the existing sorting and > iterators? Does this do the same thing, we just pass in the sort function > instead of calling traceeval_iterator_sort() one or more times? This should be used in replace of the other sorting. It allows the user to make a complex sort. If you look at the last patch, the sample uses this: +/* + * Sort all the processes by the RUNNING state. + * If A and B have the same COMM, then sort by state. + * else + * Find the RUNNNIG state for A and B + * If the RUNNING state does not exist, it's considered -1 + * If RUNNING is equal, then sort by COMM. + */ +static int compare_pdata(struct traceeval *teval_data, + const union traceeval_data *Akeys, + const union traceeval_data *Avals, + const union traceeval_data *Bkeys, + const union traceeval_data *Bvals, + void *data) +{ + struct traceeval *teval = data; /* The deltas are here */ + union traceeval_data keysA[] = { + { .cstring = Akeys[0].cstring }, { .number = RUNNING } }; + union traceeval_data keysB[] = { + { .cstring = Bkeys[0].cstring }, { .number = RUNNING } }; + struct traceeval_stat *statA; + struct traceeval_stat *statB; + unsigned long long totalA = -1; + unsigned long long totalB = -1; + + /* First check if we are on the same task */ + if (strcmp(Akeys[0].cstring, Bkeys[0].cstring) == 0) { + /* Sort decending */ + if (Bkeys[1].number > Akeys[1].number) + return -1; + return Bkeys[1].number != Akeys[1].number; + } + + /* Get the RUNNING values for both processes */ + statA = traceeval_stat(teval, keysA, &delta_vals[0]); + if (statA) + totalA = traceeval_stat_total(statA); + + statB = traceeval_stat(teval, keysB, &delta_vals[0]); + if (statB) + totalB = traceeval_stat_total(statB); + + if (totalB < totalA) + return -1; + if (totalB > totalA) + return 1; + + return strcmp(Bkeys[0].cstring, Akeys[0].cstring); +} > > > + > > + iter->needs_sort = false; > > Also probably need to set > iter->next = 0; So much for redundancy! Thanks, -- Steve > > > + return 0; > > +} > > +
diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h index 1a24d6117b93..839f63630897 100644 --- a/include/traceeval-hist.h +++ b/include/traceeval-hist.h @@ -86,6 +86,13 @@ typedef int (*traceeval_data_copy_fn)(const struct traceeval_type *type, union traceeval_data *copy, const union traceeval_data *origin); +typedef int (*traceeval_cmp_fn)(struct traceeval *teval, + const union traceeval_data *Akeys, + const union traceeval_data *Avals, + const union traceeval_data *Bkeys, + const union traceeval_data *Bvals, + void *data); + /* * struct traceeval_type - Describes the type of a traceevent_data instance * @type: The enum type that describes the traceeval_data @@ -172,6 +179,8 @@ struct traceeval_iterator *traceeval_iterator_get(struct traceeval *teval); void traceeval_iterator_put(struct traceeval_iterator *iter); int traceeval_iterator_sort(struct traceeval_iterator *iter, const char *sort_field, int level, bool ascending); +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); diff --git a/src/histograms.c b/src/histograms.c index 643a550422f6..33c87644d468 100644 --- a/src/histograms.c +++ b/src/histograms.c @@ -1153,6 +1153,40 @@ static int sort_iter(struct traceeval_iterator *iter) return 0; } +struct iter_custom_data { + struct traceeval_iterator *iter; + traceeval_cmp_fn sort_fn; + void *data; +}; + +static int iter_custom_cmp(const void *A, const void *B, void *data) +{ + struct iter_custom_data *cust_data = data; + struct traceeval_iterator *iter = cust_data->iter; + struct traceeval *teval = iter->teval; + const struct entry *a = *((const struct entry **)A); + const struct entry *b = *((const struct entry **)B); + + return cust_data->sort_fn(teval, a->keys, a->vals, b->keys, b->vals, + cust_data->data); +} + +int traceeval_iterator_sort_custom(struct traceeval_iterator *iter, + traceeval_cmp_fn sort_fn, void *data) +{ + struct iter_custom_data cust_data = { + .iter = iter, + .sort_fn = sort_fn, + .data = data + }; + + qsort_r(iter->entries, iter->nr_entries, sizeof(*iter->entries), + iter_custom_cmp, &cust_data); + + iter->needs_sort = false; + return 0; +} + /** * traceeval_iterator_next - retrieve the next entry from an iterator * @iter: The iterator to retrieve the next entry from