Re: [PATCH v1 1/3] perf diff: Support --time filter option

[Jin, Yao]

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