Messages in this thread |  | | Date | Tue, 5 Nov 2013 12:58:02 +0100 | From | Ingo Molnar <> | Subject | Re: [RFC/PATCHSET 00/14] perf report: Add support to accumulate hist periods (v2) |
| |
* Namhyung Kim <namhyung@kernel.org> wrote:
> On Tue, 5 Nov 2013 08:46:50 +0100, Ingo Molnar wrote: > > * Namhyung Kim <namhyung@kernel.org> wrote: > >> I think it'd better to separate the option and pass column and > >> (optional) sort key argument. > >> > >> --cumulative both,total (default) > >> --cumulative both,self > >> --cumulative total > >> --cumulative self (meaningless?) > >> > >> Maybe we need a config option and a single letter option for the default > >> case like --call-graph and -g options do. > >> > >> What do you think? > > > > So why restrict it to 'cumulative'? Why not have a generic --fields/-F, > > with a good default. The ordering of the fields determines sorting > > behavior. > > Ah, I didn't know you meant that too. :) > > But the 'cumulative' (btw, I feel a bit hard to type this word..) is > different in that it *generates* entries didn't get sampled originally. > And as it requires callchains, total field will not work if callchains > are missing.
Well, 'total' should disappear if it's not available.
We already have some 'column elimination/optimization' logic - like the 'dso' will disappear already if it's a single dso everywhere, IIRC?
> So I tried to make it a standalone option. > > > > > The default would be something like: > > > > -F total,self,process,dso,name > > > > Whether 'cumulative' data is calculated is not a function of any direct > > option, but simply a function of whether the 'total' field is in the -F > > list of columns displayed or not. > > So you want to turn the cumulative behavior always on, right?
Yes.
> But as Frederic noted, it might affect the performance of perf report, > so it might be better to delay this behavior to make default after users > feel comfortable with an option?
I think with call-chain speedups it should be fast enough, right?
We can argue about the default separately - if it's all done correctly then it should be really easy to change the default layout of 'perf report'.
> > With that scheme we could also do things like this to get old-style > > sorting: > > > > -F self,process,dso,name > > > > Or a really frugal 'readprofile'-style output: > > > > -F self,name > > > > if one is only interested in percentages and raw function names. > > s/name/sym(bol)/ :)
Yeah.
> Yes, this is what we do with -s option now. > > > Wrt. sorting order, by default the first column in the list of columns > > would be the primary (and only) sort key. > > Ah, I never thought it like this way. It makes sense as sort orders > really affect the output sorting. > > > (The -F field setup list could also be specified in the .perfconfig.) > > > > With this method we could do away with all this geometrical explosion > > of somewhat inconsistent formatting and sorting options... > > For now, there're two kind of columns: > > - one for showing entry's overhead percentage: self, sys, user, > guest_sys and guest_user. So the 'total' should go into this > category. I named it hpp (hist_entry period percentage) functions and > yes, I know it's an awfully bad name. :) Please see perf_hpp__format. > > There're controlled by a couple of options: --show-total-period, > --show-nr-samples and --showcpuutilization (I hate this!). And event > group also can affect its output. > > - one for grouping entries: cpu, pid, comm, dso, symbol, srcline and > parent. We call it "sort keys" but confusingly it doesn't affect > output sorting for now.
Well, it's still a sort key in a sense, a string lexicographical ordering in essence, right?
> So I think cleaning this up with -F option is good and I've been wanting > this discussion for a long time. :)
Okay :-)
> > If there's demand then we could decouple sort keys from the display > > order, by slightly augmenting the field format: > > > > -F total,self:2,process:0,dso:1,name > > > > This would sort by 'process' field as the primary key, 'dso' the secondary > > key and 'self' as the tertiary key. > > > > And we could also keep the -s/--sort option: > > > > -s process,dso,self > > > > So the above -F line would be equivalent to: > > > > -F total,self,process,dso,name -s process,dso,self > > > > What do you think? > > I like the second one. It can sustain the old way but can support the > new way easily. > > But for compatibility we need to use 'self' sort key internally iff > neither the -F option nor the config option was given by user. And it > might warn (or notice) users to add 'self' column in the sort key for > future use.
Mind explaining what the problem here is? I don't think I get it.
Thanks,
Ingo
|  |