Messages in this thread | | | From | Namhyung Kim <> | Subject | Re: [RFC/PATCHSET 00/14] perf report: Add support to accumulate hist periods (v2) | Date | Wed, 06 Nov 2013 18:17:45 +0900 |
| |
On Wed, 6 Nov 2013 09:30:46 +0100, Ingo Molnar wrote: > * Namhyung Kim <namhyung@kernel.org> wrote: > >> Hi Ingo, >> >> On Tue, 5 Nov 2013 12:58:02 +0100, Ingo Molnar wrote: >> > * Namhyung Kim <namhyung@kernel.org> wrote: >> >> 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. >> >> But what if it's the only sort key user gave? > > Do you mean something like: > > -F self,name -s total > > i.e. if a sort key not displayed?
What I worry is when no -F option was given at all.
> > I think sort keys should be automatically added to the displayed fields > list.
Agreed.
> > This rule is obviously met with the -F total:2,self:1,name:0 kind of > sorting syntax (you can only sort by fields that get displayed) - if mixed > with -s then it should be implicit I think. > >> >> 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. >> >> Well, normal users still use it as they used to - like >> 'perf report -s comm,dso' without -F option and the config. >> >> In that case, what would the output look like? According to the above >> proposal it'd look like below. >> >> # Command Shared object >> # ....... ............. >> aaa aaa >> aaa libc.so >> bbb bbb >> bbb libc.so >> >> >> But the user might want see this: >> >> # Overhead (self) Command Shared object >> # ............... ....... ............. >> 30.00% bbb bbb >> 25.00% aaa aaa >> 25.00% aaa libc.so >> 20.00% bbb libc.so >> >> >> If she really wants to see it sorted by comm and dso, the command line >> should be 'perf report -F self,comm,dso -s comm,dso' >> (or just 'perf report -F self -s comm,dso' could do the same). >> >> # Overhead (self) Command Shared object >> # ............... ....... ............. >> 25.00% aaa aaa >> 25.00% aaa libc.so >> 30.00% bbb bbb >> 20.00% bbb libc.so > > This problem should be solved if all -s fields are displayed - i.e. they > are added to the -F list, right?
But old users might not aware of the new -F option, and use -s option only. If so, she will get output like the first example, right?
> > Basically there's just a single concept: the -F list. The -s option simply > modifies and extends the -F list but internally perf report would not know > anything about '-s', it only knows about fields to display and it would > know which of those fields are to be sorted and in what order. > > Does that make sense to you? Does it cover everything needed?
I like the concept. I'm just looking for a way to add it without upsetting old users. :)
Thanks, Namhyung
| |