Messages in this thread | | | Subject | Re: [PATCH v1 1/3] perf diff: Support --time filter option | From | "Jin, Yao" <> | Date | Mon, 25 Feb 2019 22:12:08 +0800 |
| |
On 2/25/2019 9:38 PM, Jiri Olsa wrote: > On Mon, Feb 25, 2019 at 09:42:42PM +0800, Jin Yao wrote: > > SNIP > >> COMPARISON >> ---------- >> The comparison is governed by the baseline file. The baseline perf.data >> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c >> index 751e197..ddc41e7 100644 >> --- a/tools/perf/builtin-diff.c >> +++ b/tools/perf/builtin-diff.c >> @@ -19,12 +19,21 @@ >> #include "util/util.h" >> #include "util/data.h" >> #include "util/config.h" >> +#include "util/time-utils.h" >> >> #include <errno.h> >> #include <inttypes.h> >> #include <stdlib.h> >> #include <math.h> >> >> +struct perf_diff { >> + struct perf_tool tool; >> + const char *time_str; >> + struct perf_time_interval *ptime_range; >> + int range_size; >> + int range_num; > > please align the members >
Not aligned? Sorry, could you give me an example?
>> static int __cmd_diff(void) >> { >> struct data__file *d; >> int ret = -EINVAL, i; >> + char *abstime_ostr = NULL, *abstime_tmp = NULL; >> + bool abstime = false; >> + >> + if (pdiff.time_str && strchr(pdiff.time_str, ':')) { >> + abstime = true; >> + pdiff.ptime_range = perf_time__range_alloc(NULL, >> + &pdiff.range_size); >> + } else { >> + pdiff.ptime_range = perf_time__range_alloc(pdiff.time_str, >> + &pdiff.range_size); >> + } >> + >> + if (!pdiff.ptime_range) >> + return -ENOMEM; >> + >> + if (abstime) { >> + abstime_ostr = strdup(pdiff.time_str); >> + if (!abstime_ostr) { >> + zfree(&pdiff.ptime_range); >> + return -ENOMEM; >> + } >> + abstime_tmp = abstime_ostr; >> + } > > could you please put this in separate function? > >
Sure, I will put this in a separate function.
>> >> data__for_each_file(i, d) { >> - d->session = perf_session__new(&d->data, false, &tool); >> + d->session = perf_session__new(&d->data, false, &pdiff.tool); >> if (!d->session) { >> pr_err("Failed to open %s\n", d->data.file.path); >> ret = -1; >> goto out_delete; >> } >> >> + if (abstime) { >> + ret = parse_absolute_time(d, &abstime_tmp); >> + if (ret < 0) >> + goto out_delete; >> + } else if (pdiff.time_str) { >> + ret = parse_percent_time(d); >> + if (ret < 0) >> + goto out_delete; >> + } else { >> + pdiff.range_num = 1; >> + } >> + > > and this as well >
OK.
Thanks Jin Yao
> thanks, > jirka >
| |