Messages in this thread | | | Date | Mon, 2 Dec 2013 16:23:15 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 3/3] perf report: Add --show-time-info option |
| |
Em Mon, Dec 02, 2013 at 11:38:20PM +0900, Namhyung Kim escreveu: > Hi Arnaldo, > > 2013-12-02 (월), 09:33 -0300, Arnaldo Carvalho de Melo: > > Em Mon, Dec 02, 2013 at 03:53:19PM +0900, Namhyung Kim escreveu: > > > From: Namhyung Kim <namhyung.kim@lge.com> > > > > > > The --show-time-info option is for displaying elapsed sampling time > > > > Isn't that too long? Can't we use just --time/-t? > > Hmm.. interesting. Your previous feedback was to use longer and clearer > option name. :)
> https://lkml.org/lkml/2013/11/18/198
Hey, longer and more descriptive names _when required_, in that case I thought that even being longer --show-task-events would be better, but here using --show-time-info the 'info' part looks superfluous, at least to me, hence my suggestion/question :-)
<SNIP>
> > > +++ b/tools/perf/ui/hist.c > > > @@ -7,19 +7,24 @@ > > > #include "../util/evsel.h" > > > > > > /* hist period print (hpp) functions */ > > > +enum hpp_fmt_type { > > > + HPP_FMT__PERCENT, > > > + HPP_FMT__RAW, > > > + HPP_FMT__TIME, > > > +}; > > > > I wonder if we can't make this a bitmask... Does it make sense to print > > both percent and time, for instance? > > I'm not sure I understood you correctly, but did you mean printing both > percent and time in a single column? This enum and the __hpp_fmt() > function is for a single column. I can add a new column for time > percent if you want.
I have not delved into the code, it was just a first impression, lemme do that now...
- Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |