lkml.org 
[lkml]   [2019]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/3] perf diff: Support --time filter option
On Wed, Feb 27, 2019 at 09:04:21PM +0800, Jin, Yao wrote:
>
>
> On 2/27/2019 5:27 PM, Jiri Olsa wrote:
> > On Tue, Feb 26, 2019 at 08:11:07PM +0800, Jin Yao wrote:
> >
> > SNIP
> >
> > > + abstime_tmp = abstime_ostr;
> > > 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.path);
> > > ret = -1;
> > > goto out_delete;
> > > }
> > > + if (abstime_ostr) {
> > > + 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;
> >
> > hum, why are we setting range_num to 1 again?
>
> Yes, that may be not necessary. I will remove this line and test again.
>
> >
> > it's really hard to parse this code, maybe
> > it'd be better in separate loop/function
> > that would setup just timestamps..
> >
>
> Do you mean the above parsing code should be put in a separate function
> (e.g. parse_time_string in following example)?
>
> data__for_each_file(i, d) {
> ....
> d->session = perf_session__new(&d->data, false, &pdiff.tool);
> ....
> parse_time_string(...);
> ret = perf_session__process_events(d->session);
> ....
> }

anything that would make this more clear/readable ;-)

thanks,
jirka

\
 
 \ /
  Last update: 2019-02-27 14:26    [W:0.037 / U:10.232 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site