Re: [PATCH 12/18] perf ui/hist: Add support for event group view
Hi Arnaldo, On Tue, 4 Dec 2012 10:50:49 -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, Dec 04, 2012 at 06:16:59PM +0900, Namhyung Kim escreveu: >> On Fri, Nov 30, 2012 at 10:52 PM, Arnaldo Carvalho de Melo wrote: > >> Ah, I missed your point. Just got it now, will try this approach. So >> you want to see no "0.00%" for a dummy entry, right? > > That wasn't the point, and perhaps printing 0.00% pollutes the screen > needlessly or may be a help in seeing the columns more clearly, no > strong opinion at this point, please experiment. > > The point was that there seems to be no need for the temporary array. Yeah, it shows my really bad writing skill. :( I meant I got your point of not using a temporary array. And asked additional question of your preference in a dummy entry. Sorry for confusing you. Btw, I found that printing 0.00% is useful for a case of field_sep and GTK+ output. So unless someone objects, I'll keep it. Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
Em Tue, Dec 04, 2012 at 06:16:59PM +0900, Namhyung Kim escreveu: > On Fri, Nov 30, 2012 at 10:52 PM, Arnaldo Carvalho de Melo wrote: > Ah, I missed your point. Just got it now, will try this approach. So > you want to see no "0.00%" for a dummy entry, right? That wasn't the point, and perhaps printing 0.00% pollutes the screen needlessly or may be a help in seeing the columns more clearly, no strong opinion at this point, please experiment. The point was that there seems to be no need for the temporary array. - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Fri, Nov 30, 2012 at 10:52 PM, Arnaldo Carvalho de Melo
wrote:
> Em Fri, Nov 30, 2012 at 02:29:43PM +0100, Jiri Olsa escreveu:
>> On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
>> > +#define __HPP_COLOR_PERCENT_FN(_type, _field)
>> > \
>> > +static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry
>> > *he) \
>> > +{
>> > \
>> > + int ret;
>> > \
>> > + double percent = 0.0;
>> > \
>> > + struct hists *hists = he->hists;
>> > \
>> > +
>> > \
>> > + if (hists->stats.total_period)
>> > \
>> > + percent = 100.0 * he->stat._field / hists->stats.total_period;
>> > \
>> > +
>> > \
>> > + ret = percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%",
>> > percent); \
>> > +
>> > \
>> > + if (symbol_conf.event_group) {
>> > \
>> > + int i;
>> > \
>> > + struct perf_evsel *evsel = hists_to_evsel(hists);
>> > \
>> > + struct hist_entry *pair;
>> > \
>> > + int nr_members = evsel->nr_members;
>> > \
>> > + double *percents;
>> > \
>> > +
>> > \
>> > + if (nr_members <= 1)
>> > \
>> > + return ret;
>> > \
>> > +
>> > \
>> > + percents = zalloc(sizeof(*percents) * nr_members);
>> > \
>> > + if (percents == NULL) {
>> > \
>> > + pr_warning("Not enough memory!\n");
>> > \
>> > + return ret;
>> > \
>> > + }
>> > \
>
> Why do we need to zalloc this percents array?
>
>> > + list_for_each_entry(pair, >pairs.head, pairs.node) {
>> > \
>> > + u64 period = pair->stat._field;
>> > \
>> > + u64 total = pair->hists->stats.total_period;
>> > \
>> > +
>> > \
>> > + if (!total)
>> > \
>> > + continue;
>> > \
>> > +
>> > \
>> > + evsel = hists_to_evsel(pair->hists);
>> > \
>> > + i = perf_evsel__group_idx(evsel);
>> > \
>> > + percents[i] = 100.0 * period / total;
>> > \
>
> Why not use percent_color_snprintf here, using some "%*s" thing that
> uses i to position the output in the right column? This way the
> following loop can be ditched as well. No?
>
>> > + }
>> > \
>> > +
>> > \
>> > + for (i = 1; i < nr_members; i++) {
>> > \
>> > + ret += percent_color_snprintf(hpp->buf + ret,
>> > \
>> > + hpp->size - ret,
>> > \
>> > + " %6.2f%%",
>> > percents[i]); \
>> > + }
>> > \
>> > + free(percents);
>> > \
>> > + }
>> > \
>> > + return ret;
>> > \
>
> - Arnaldo
Ah, I missed your point. Just got it now, will try this approach. So
you want to see no "0.00%" for a dummy entry, right?
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Fri, Nov 30, 2012 at 10:52 PM, Arnaldo Carvalho de Melo
a...@ghostprotocols.net wrote:
Em Fri, Nov 30, 2012 at 02:29:43PM +0100, Jiri Olsa escreveu:
On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
+#define __HPP_COLOR_PERCENT_FN(_type, _field)
\
+static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry
*he) \
+{
\
+ int ret;
\
+ double percent = 0.0;
\
+ struct hists *hists = he-hists;
\
+
\
+ if (hists-stats.total_period)
\
+ percent = 100.0 * he-stat._field / hists-stats.total_period;
\
+
\
+ ret = percent_color_snprintf(hpp-buf, hpp-size, %6.2f%%,
percent); \
+
\
+ if (symbol_conf.event_group) {
\
+ int i;
\
+ struct perf_evsel *evsel = hists_to_evsel(hists);
\
+ struct hist_entry *pair;
\
+ int nr_members = evsel-nr_members;
\
+ double *percents;
\
+
\
+ if (nr_members = 1)
\
+ return ret;
\
+
\
+ percents = zalloc(sizeof(*percents) * nr_members);
\
+ if (percents == NULL) {
\
+ pr_warning(Not enough memory!\n);
\
+ return ret;
\
+ }
\
Why do we need to zalloc this percents array?
+ list_for_each_entry(pair, he-pairs.head, pairs.node) {
\
+ u64 period = pair-stat._field;
\
+ u64 total = pair-hists-stats.total_period;
\
+
\
+ if (!total)
\
+ continue;
\
+
\
+ evsel = hists_to_evsel(pair-hists);
\
+ i = perf_evsel__group_idx(evsel);
\
+ percents[i] = 100.0 * period / total;
\
Why not use percent_color_snprintf here, using some %*s thing that
uses i to position the output in the right column? This way the
following loop can be ditched as well. No?
+ }
\
+
\
+ for (i = 1; i nr_members; i++) {
\
+ ret += percent_color_snprintf(hpp-buf + ret,
\
+ hpp-size - ret,
\
+ %6.2f%%,
percents[i]); \
+ }
\
+ free(percents);
\
+ }
\
+ return ret;
\
- Arnaldo
Ah, I missed your point. Just got it now, will try this approach. So
you want to see no 0.00% for a dummy entry, right?
Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
Em Tue, Dec 04, 2012 at 06:16:59PM +0900, Namhyung Kim escreveu: On Fri, Nov 30, 2012 at 10:52 PM, Arnaldo Carvalho de Melo wrote: Ah, I missed your point. Just got it now, will try this approach. So you want to see no 0.00% for a dummy entry, right? That wasn't the point, and perhaps printing 0.00% pollutes the screen needlessly or may be a help in seeing the columns more clearly, no strong opinion at this point, please experiment. The point was that there seems to be no need for the temporary array. - Arnaldo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
Hi Arnaldo, On Tue, 4 Dec 2012 10:50:49 -0300, Arnaldo Carvalho de Melo wrote: Em Tue, Dec 04, 2012 at 06:16:59PM +0900, Namhyung Kim escreveu: On Fri, Nov 30, 2012 at 10:52 PM, Arnaldo Carvalho de Melo wrote: Ah, I missed your point. Just got it now, will try this approach. So you want to see no 0.00% for a dummy entry, right? That wasn't the point, and perhaps printing 0.00% pollutes the screen needlessly or may be a help in seeing the columns more clearly, no strong opinion at this point, please experiment. The point was that there seems to be no need for the temporary array. Yeah, it shows my really bad writing skill. :( I meant I got your point of not using a temporary array. And asked additional question of your preference in a dummy entry. Sorry for confusing you. Btw, I found that printing 0.00% is useful for a case of field_sep and GTK+ output. So unless someone objects, I'll keep it. Thanks, Namhyung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Mon, 3 Dec 2012 16:57:36 +0100, Jiri Olsa wrote: > On Mon, Dec 03, 2012 at 07:39:31PM +0900, Namhyung Kim wrote: >> On Mon, 3 Dec 2012 11:23:27 +0100, Jiri Olsa wrote: >> > On Mon, Dec 03, 2012 at 10:56:28AM +0900, Namhyung Kim wrote: >> >> On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa wrote: >> >> > ok, so this is the part thats common for both multi diff and group >> >> > report and hugely depends on how we link matching hist_entry > > SNIP > >> > we discussed with Arnaldo and decided to stay with current approach and >> > make the change later if needed.. to be able to see/meassure the benefit >> > >> > I made some initial attempt to workaround this and it appears to be >> > not that bad ;) I'll repost my changes shortly.. >> >> Hmm.. so are you OK with this patch now? > > yep, well except for following macros: > > __HPP_COLOR_PERCENT_FN > __HPP_ENTRY_PERCENT_FN > __HPP_ENTRY_RAW_FN > > being so long.. any chance the main part of it being in function? > > Seems like '_type' is just in function name, but the '_field' might > be the killer ;) maybe it could be 'used' in such function via PERF_HPP__* > enums instead..? Okay, I'll re-think about it tomorrow. Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Mon, Dec 03, 2012 at 07:39:31PM +0900, Namhyung Kim wrote: > On Mon, 3 Dec 2012 11:23:27 +0100, Jiri Olsa wrote: > > On Mon, Dec 03, 2012 at 10:56:28AM +0900, Namhyung Kim wrote: > >> On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa wrote: > >> > ok, so this is the part thats common for both multi diff and group > >> > report and hugely depends on how we link matching hist_entry SNIP > > we discussed with Arnaldo and decided to stay with current approach and > > make the change later if needed.. to be able to see/meassure the benefit > > > > I made some initial attempt to workaround this and it appears to be > > not that bad ;) I'll repost my changes shortly.. > > Hmm.. so are you OK with this patch now? yep, well except for following macros: __HPP_COLOR_PERCENT_FN __HPP_ENTRY_PERCENT_FN __HPP_ENTRY_RAW_FN being so long.. any chance the main part of it being in function? Seems like '_type' is just in function name, but the '_field' might be the killer ;) maybe it could be 'used' in such function via PERF_HPP__* enums instead..? thanks, jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Mon, 3 Dec 2012 11:23:27 +0100, Jiri Olsa wrote: > On Mon, Dec 03, 2012 at 10:56:28AM +0900, Namhyung Kim wrote: >> On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa wrote: >> > ok, so this is the part thats common for both multi diff and group >> > report and hugely depends on how we link matching hist_entry >> > >> > To sum to what group report does here: >> > >> > 1) starting with event group >> > 2) the function 'he' belongs to leader hists >> > 3) display leaders data value >> > 4) if 'symbol_conf.event_group' is enabled, we want to display all >> >group members data values in a column >> > 5) say we have group of 3 events 'e1,e2,e3' and e1 being the leader, >> >we have following possibilities: >> >- e1 have no pairs >> >- e1 is paired with e2 >> >- e1 is paired with e3 >> >- e1 is paired with e2 and e3 (e1 could also be dummy one) >> > 6) need to display 3 values wrt to e1,e2,e3 output order >> > 7) because events belongs to a group, each evsel carries group idx >> > 8) so we walk all pairs and compute the eX value and put it >> >into temporary array based on its group idx >> > 9) finally we display all temporary array values >> > >> > >> > To sum up what multi diff needs to do here: >> > >> > 1) starting with 3 separate matching events from different >> >evlists(sessions) >> > 2 - 5) are similar >> > 6) need to display single diff value of hist_entry that >> >belongs to evsel, that belongs to a column we are just >> >displaying value for >> > 7) events are not part of group; based on >> >[PATCH 02/14] perf tool: Add struct perf_hpp_fmt into hpp callbacks >> >we can tell what column we are in -> session -> evlist >> > 8) we need to walk all pairs and for each hist_entry: >> >- compare all evsels (from point 7 evlist) >> > and match hists pointer >> >- when found, we have a matching hist_entry for this column >> > 9) print out the value of matching hist_entry >> > >> > >> > I think both this processing could be simplified by having hist_entry >> > pairs connected via array and not linked list. >> > >> > >> > For group report, each leader hist_entry would have 'pairs' array >> > with the size of event group. Then we could omit the temporary array >> > creation by: >> > - walking the leaders pairs >> > - when pair is found -> compute data -> display >> > - when pair is NULL -> display 0 >> > >> > >> > For multi diff, each baseline hist_entry would have 'pairs' array >> > with the size equal to number of data files on diff command input. >> > This way we could use the data from point 7 to directly access >> > related hist_entry. >> > >> > >> > ufff... thoughts? ;-) >> >> Nice summary, really! :) >> >> Yeah, I do think it's better to use array for this. If Arnaldo has no >> objection to this approach, I'll convert my code to use the array. > > we discussed with Arnaldo and decided to stay with current approach and > make the change later if needed.. to be able to see/meassure the benefit > > I made some initial attempt to workaround this and it appears to be > not that bad ;) I'll repost my changes shortly.. Hmm.. so are you OK with this patch now? Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Mon, Dec 03, 2012 at 10:56:28AM +0900, Namhyung Kim wrote:
> Hi Jiri,
>
> On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa wrote:
> > On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
> >
> > SNIP
> >
> >> +#define __HPP_COLOR_PERCENT_FN(_type, _field)
> >>\
> >> +static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry
> >> *he) \
> >> +{
> >>\
> >> + int ret;
> >>\
> >> + double percent = 0.0;
> >>\
> >> + struct hists *hists = he->hists;
> >>\
> >> +
> >>\
> >> + if (hists->stats.total_period)
> >>\
> >> + percent = 100.0 * he->stat._field /
> >> hists->stats.total_period; \
> >> +
> >>\
> >> + ret = percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%",
> >> percent); \
> >> +
> >>\
> >> + if (symbol_conf.event_group) {
> >>\
> >> + int i;
> >>\
> >> + struct perf_evsel *evsel = hists_to_evsel(hists);
> >>\
> >> + struct hist_entry *pair;
> >>\
> >> + int nr_members = evsel->nr_members;
> >>\
> >> + double *percents;
> >>\
> >> +
> >>\
> >> + if (nr_members <= 1)
> >>\
> >> + return ret;
> >>\
> >> +
> >>\
> >> + percents = zalloc(sizeof(*percents) * nr_members);
> >>\
> >> + if (percents == NULL) {
> >>\
> >> + pr_warning("Not enough memory!\n");
> >>\
> >> + return ret;
> >>\
> >> + }
> >>\
> >> +
> >>\
> >> + list_for_each_entry(pair, >pairs.head, pairs.node) {
> >>\
> >> + u64 period = pair->stat._field;
> >>\
> >> + u64 total = pair->hists->stats.total_period;
> >>\
> >> +
> >>\
> >> + if (!total)
> >>\
> >> + continue;
> >>\
> >> +
> >>\
> >> + evsel = hists_to_evsel(pair->hists);
> >>\
> >> + i = perf_evsel__group_idx(evsel);
> >>\
> >> + percents[i] = 100.0 * period / total;
> >>\
> >> + }
> >>\
> >> +
> >>\
> >> + for (i = 1; i < nr_members; i++) {
> >>\
> >> + ret += percent_color_snprintf(hpp->buf + ret,
> >>\
> >> + hpp->size - ret,
> >>\
> >> + " %6.2f%%",
> >> percents[i]); \
> >> + }
> >>\
> >> + free(percents);
> >>\
> >> + }
> >>\
> >> + return ret;
> >>\
> >> +}
> >
> > ok, so this is the part thats common for both multi diff and group
> > report and hugely depends on how we link matching hist_entry
> >
> > To sum to what group report does here:
> >
> > 1) starting with event group
> > 2) the function 'he' belongs to leader hists
> > 3) display leaders data value
> > 4) if 'symbol_conf.event_group' is enabled, we want to display all
> >group members data
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Mon, Dec 03, 2012 at 10:56:28AM +0900, Namhyung Kim wrote:
Hi Jiri,
On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa jo...@redhat.com wrote:
On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
SNIP
+#define __HPP_COLOR_PERCENT_FN(_type, _field)
\
+static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry
*he) \
+{
\
+ int ret;
\
+ double percent = 0.0;
\
+ struct hists *hists = he-hists;
\
+
\
+ if (hists-stats.total_period)
\
+ percent = 100.0 * he-stat._field /
hists-stats.total_period; \
+
\
+ ret = percent_color_snprintf(hpp-buf, hpp-size, %6.2f%%,
percent); \
+
\
+ if (symbol_conf.event_group) {
\
+ int i;
\
+ struct perf_evsel *evsel = hists_to_evsel(hists);
\
+ struct hist_entry *pair;
\
+ int nr_members = evsel-nr_members;
\
+ double *percents;
\
+
\
+ if (nr_members = 1)
\
+ return ret;
\
+
\
+ percents = zalloc(sizeof(*percents) * nr_members);
\
+ if (percents == NULL) {
\
+ pr_warning(Not enough memory!\n);
\
+ return ret;
\
+ }
\
+
\
+ list_for_each_entry(pair, he-pairs.head, pairs.node) {
\
+ u64 period = pair-stat._field;
\
+ u64 total = pair-hists-stats.total_period;
\
+
\
+ if (!total)
\
+ continue;
\
+
\
+ evsel = hists_to_evsel(pair-hists);
\
+ i = perf_evsel__group_idx(evsel);
\
+ percents[i] = 100.0 * period / total;
\
+ }
\
+
\
+ for (i = 1; i nr_members; i++) {
\
+ ret += percent_color_snprintf(hpp-buf + ret,
\
+ hpp-size - ret,
\
+%6.2f%%,
percents[i]); \
+ }
\
+ free(percents);
\
+ }
\
+ return ret;
\
+}
ok, so this is the part thats common for both multi diff and group
report and hugely depends on how we link matching hist_entry
To sum to what group report does here:
1) starting with event group
2) the function 'he' belongs to leader hists
3) display leaders data value
4) if 'symbol_conf.event_group' is enabled, we want to display all
group members data values in a column
5) say we have group of 3 events 'e1,e2,e3' and e1 being the leader,
we have following possibilities:
- e1 have no pairs
- e1 is paired with e2
- e1 is paired with e3
- e1 is paired with e2 and e3 (e1 could also be dummy one)
6) need to display 3 values wrt to e1,e2,e3 output
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Mon, 3 Dec 2012 11:23:27 +0100, Jiri Olsa wrote: On Mon, Dec 03, 2012 at 10:56:28AM +0900, Namhyung Kim wrote: On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa jo...@redhat.com wrote: ok, so this is the part thats common for both multi diff and group report and hugely depends on how we link matching hist_entry To sum to what group report does here: 1) starting with event group 2) the function 'he' belongs to leader hists 3) display leaders data value 4) if 'symbol_conf.event_group' is enabled, we want to display all group members data values in a column 5) say we have group of 3 events 'e1,e2,e3' and e1 being the leader, we have following possibilities: - e1 have no pairs - e1 is paired with e2 - e1 is paired with e3 - e1 is paired with e2 and e3 (e1 could also be dummy one) 6) need to display 3 values wrt to e1,e2,e3 output order 7) because events belongs to a group, each evsel carries group idx 8) so we walk all pairs and compute the eX value and put it into temporary array based on its group idx 9) finally we display all temporary array values To sum up what multi diff needs to do here: 1) starting with 3 separate matching events from different evlists(sessions) 2 - 5) are similar 6) need to display single diff value of hist_entry that belongs to evsel, that belongs to a column we are just displaying value for 7) events are not part of group; based on [PATCH 02/14] perf tool: Add struct perf_hpp_fmt into hpp callbacks we can tell what column we are in - session - evlist 8) we need to walk all pairs and for each hist_entry: - compare all evsels (from point 7 evlist) and match hists pointer - when found, we have a matching hist_entry for this column 9) print out the value of matching hist_entry I think both this processing could be simplified by having hist_entry pairs connected via array and not linked list. For group report, each leader hist_entry would have 'pairs' array with the size of event group. Then we could omit the temporary array creation by: - walking the leaders pairs - when pair is found - compute data - display - when pair is NULL - display 0 For multi diff, each baseline hist_entry would have 'pairs' array with the size equal to number of data files on diff command input. This way we could use the data from point 7 to directly access related hist_entry. ufff... thoughts? ;-) Nice summary, really! :) Yeah, I do think it's better to use array for this. If Arnaldo has no objection to this approach, I'll convert my code to use the array. we discussed with Arnaldo and decided to stay with current approach and make the change later if needed.. to be able to see/meassure the benefit I made some initial attempt to workaround this and it appears to be not that bad ;) I'll repost my changes shortly.. Hmm.. so are you OK with this patch now? Thanks, Namhyung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Mon, Dec 03, 2012 at 07:39:31PM +0900, Namhyung Kim wrote: On Mon, 3 Dec 2012 11:23:27 +0100, Jiri Olsa wrote: On Mon, Dec 03, 2012 at 10:56:28AM +0900, Namhyung Kim wrote: On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa jo...@redhat.com wrote: ok, so this is the part thats common for both multi diff and group report and hugely depends on how we link matching hist_entry SNIP we discussed with Arnaldo and decided to stay with current approach and make the change later if needed.. to be able to see/meassure the benefit I made some initial attempt to workaround this and it appears to be not that bad ;) I'll repost my changes shortly.. Hmm.. so are you OK with this patch now? yep, well except for following macros: __HPP_COLOR_PERCENT_FN __HPP_ENTRY_PERCENT_FN __HPP_ENTRY_RAW_FN being so long.. any chance the main part of it being in function? Seems like '_type' is just in function name, but the '_field' might be the killer ;) maybe it could be 'used' in such function via PERF_HPP__* enums instead..? thanks, jirka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Mon, 3 Dec 2012 16:57:36 +0100, Jiri Olsa wrote: On Mon, Dec 03, 2012 at 07:39:31PM +0900, Namhyung Kim wrote: On Mon, 3 Dec 2012 11:23:27 +0100, Jiri Olsa wrote: On Mon, Dec 03, 2012 at 10:56:28AM +0900, Namhyung Kim wrote: On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa jo...@redhat.com wrote: ok, so this is the part thats common for both multi diff and group report and hugely depends on how we link matching hist_entry SNIP we discussed with Arnaldo and decided to stay with current approach and make the change later if needed.. to be able to see/meassure the benefit I made some initial attempt to workaround this and it appears to be not that bad ;) I'll repost my changes shortly.. Hmm.. so are you OK with this patch now? yep, well except for following macros: __HPP_COLOR_PERCENT_FN __HPP_ENTRY_PERCENT_FN __HPP_ENTRY_RAW_FN being so long.. any chance the main part of it being in function? Seems like '_type' is just in function name, but the '_field' might be the killer ;) maybe it could be 'used' in such function via PERF_HPP__* enums instead..? Okay, I'll re-think about it tomorrow. Thanks, Namhyung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
Hi Jiri,
On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa wrote:
> On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
>
> SNIP
>
>> +#define __HPP_COLOR_PERCENT_FN(_type, _field)
>> \
>> +static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry *he)
>> \
>> +{
>> \
>> + int ret;
>> \
>> + double percent = 0.0;
>> \
>> + struct hists *hists = he->hists;
>> \
>> +
>> \
>> + if (hists->stats.total_period)
>> \
>> + percent = 100.0 * he->stat._field / hists->stats.total_period;
>> \
>> +
>> \
>> + ret = percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%",
>> percent); \
>> +
>> \
>> + if (symbol_conf.event_group) {
>> \
>> + int i;
>> \
>> + struct perf_evsel *evsel = hists_to_evsel(hists);
>> \
>> + struct hist_entry *pair;
>> \
>> + int nr_members = evsel->nr_members;
>> \
>> + double *percents;
>> \
>> +
>> \
>> + if (nr_members <= 1)
>> \
>> + return ret;
>> \
>> +
>> \
>> + percents = zalloc(sizeof(*percents) * nr_members);
>> \
>> + if (percents == NULL) {
>> \
>> + pr_warning("Not enough memory!\n");
>> \
>> + return ret;
>> \
>> + }
>> \
>> +
>> \
>> + list_for_each_entry(pair, >pairs.head, pairs.node) {
>> \
>> + u64 period = pair->stat._field;
>> \
>> + u64 total = pair->hists->stats.total_period;
>> \
>> +
>> \
>> + if (!total)
>> \
>> + continue;
>> \
>> +
>> \
>> + evsel = hists_to_evsel(pair->hists);
>> \
>> + i = perf_evsel__group_idx(evsel);
>> \
>> + percents[i] = 100.0 * period / total;
>> \
>> + }
>> \
>> +
>> \
>> + for (i = 1; i < nr_members; i++) {
>> \
>> + ret += percent_color_snprintf(hpp->buf + ret,
>> \
>> + hpp->size - ret,
>> \
>> + " %6.2f%%",
>> percents[i]); \
>> + }
>> \
>> + free(percents);
>> \
>> + }
>> \
>> + return ret;
>> \
>> +}
>
> ok, so this is the part thats common for both multi diff and group
> report and hugely depends on how we link matching hist_entry
>
> To sum to what group report does here:
>
> 1) starting with event group
> 2) the function 'he' belongs to leader hists
> 3) display leaders data value
> 4) if 'symbol_conf.event_group' is enabled, we want to display all
>group members data values in a column
> 5) say we have group of 3 events 'e1,e2,e3' and e1 being the leader,
>we have following possibilities:
>- e1 have no pairs
>- e1 is paired with e2
>- e1 is paired with e3
>- e1 is paired with e2 and e3 (e1 could also be dummy one)
> 6) need to display
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Fri, 30 Nov 2012 10:52:15 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 30, 2012 at 02:29:43PM +0100, Jiri Olsa escreveu:
>> On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
>> > +#define __HPP_COLOR_PERCENT_FN(_type, _field)
>> > \
>> > +static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry
>> > *he)\
>> > +{
>> > \
>> > + int ret;
>> > \
>> > + double percent = 0.0;
>> > \
>> > + struct hists *hists = he->hists;
>> > \
>> > +
>> > \
>> > + if (hists->stats.total_period)
>> > \
>> > + percent = 100.0 * he->stat._field / hists->stats.total_period;
>> > \
>> > +
>> > \
>> > + ret = percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
>> > \
>> > +
>> > \
>> > + if (symbol_conf.event_group) {
>> > \
>> > + int i;
>> > \
>> > + struct perf_evsel *evsel = hists_to_evsel(hists);
>> > \
>> > + struct hist_entry *pair;
>> > \
>> > + int nr_members = evsel->nr_members;
>> > \
>> > + double *percents;
>> > \
>> > +
>> > \
>> > + if (nr_members <= 1)
>> > \
>> > + return ret;
>> > \
>> > +
>> > \
>> > + percents = zalloc(sizeof(*percents) * nr_members);
>> > \
>> > + if (percents == NULL) {
>> > \
>> > + pr_warning("Not enough memory!\n");
>> > \
>> > + return ret;
>> > \
>> > + }
>> > \
>
> Why do we need to zalloc this percents array?
>
>> > + list_for_each_entry(pair, >pairs.head, pairs.node) {
>> > \
>> > + u64 period = pair->stat._field;
>> > \
>> > + u64 total = pair->hists->stats.total_period;
>> > \
>> > +
>> > \
>> > + if (!total)
>> > \
>> > + continue;
>> > \
>> > +
>> > \
>> > + evsel = hists_to_evsel(pair->hists);
>> > \
>> > + i = perf_evsel__group_idx(evsel);
>> > \
>> > + percents[i] = 100.0 * period / total;
>> > \
>
> Why not use percent_color_snprintf here, using some "%*s" thing that
> uses i to position the output in the right column? This way the
> following loop can be ditched as well. No?
It's because it's possible that an entry didn't have pairs for every
group member. Say, there's a group consists of 3 members (1 leader + 2
member). It's quite legitimate that a leader hist entry has just one
pair for a member and miss another. So I allocated a zero-filled array,
and filled what it has, and print them all in the for loop below.
Thanks,
Namhyung
>
>> > + }
>> > \
>> > +
>> > \
>> > + for (i = 1; i < nr_members; i++) {
>> > \
>> > + ret += percent_color_snprintf(hpp->buf + ret,
>> > \
>> > +hpp->size - ret,
>> > \
>> > +" %6.2f%%", percents[i]);
>> > \
>> > + }
>> > \
>> > + free(percents);
>> > \
>> > + }
>> > \
>> > + return ret;
>> > \
>
> - Arnaldo
--
To unsubscribe from this
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
On Fri, 30 Nov 2012 10:52:15 -0300, Arnaldo Carvalho de Melo wrote:
Em Fri, Nov 30, 2012 at 02:29:43PM +0100, Jiri Olsa escreveu:
On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
+#define __HPP_COLOR_PERCENT_FN(_type, _field)
\
+static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry
*he)\
+{
\
+ int ret;
\
+ double percent = 0.0;
\
+ struct hists *hists = he-hists;
\
+
\
+ if (hists-stats.total_period)
\
+ percent = 100.0 * he-stat._field / hists-stats.total_period;
\
+
\
+ ret = percent_color_snprintf(hpp-buf, hpp-size, %6.2f%%, percent);
\
+
\
+ if (symbol_conf.event_group) {
\
+ int i;
\
+ struct perf_evsel *evsel = hists_to_evsel(hists);
\
+ struct hist_entry *pair;
\
+ int nr_members = evsel-nr_members;
\
+ double *percents;
\
+
\
+ if (nr_members = 1)
\
+ return ret;
\
+
\
+ percents = zalloc(sizeof(*percents) * nr_members);
\
+ if (percents == NULL) {
\
+ pr_warning(Not enough memory!\n);
\
+ return ret;
\
+ }
\
Why do we need to zalloc this percents array?
+ list_for_each_entry(pair, he-pairs.head, pairs.node) {
\
+ u64 period = pair-stat._field;
\
+ u64 total = pair-hists-stats.total_period;
\
+
\
+ if (!total)
\
+ continue;
\
+
\
+ evsel = hists_to_evsel(pair-hists);
\
+ i = perf_evsel__group_idx(evsel);
\
+ percents[i] = 100.0 * period / total;
\
Why not use percent_color_snprintf here, using some %*s thing that
uses i to position the output in the right column? This way the
following loop can be ditched as well. No?
It's because it's possible that an entry didn't have pairs for every
group member. Say, there's a group consists of 3 members (1 leader + 2
member). It's quite legitimate that a leader hist entry has just one
pair for a member and miss another. So I allocated a zero-filled array,
and filled what it has, and print them all in the for loop below.
Thanks,
Namhyung
+ }
\
+
\
+ for (i = 1; i nr_members; i++) {
\
+ ret += percent_color_snprintf(hpp-buf + ret,
\
+hpp-size - ret,
\
+ %6.2f%%, percents[i]);
\
+ }
\
+ free(percents);
\
+ }
\
+ return ret;
\
- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
Hi Jiri,
On Fri, Nov 30, 2012 at 10:29 PM, Jiri Olsa jo...@redhat.com wrote:
On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
SNIP
+#define __HPP_COLOR_PERCENT_FN(_type, _field)
\
+static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry *he)
\
+{
\
+ int ret;
\
+ double percent = 0.0;
\
+ struct hists *hists = he-hists;
\
+
\
+ if (hists-stats.total_period)
\
+ percent = 100.0 * he-stat._field / hists-stats.total_period;
\
+
\
+ ret = percent_color_snprintf(hpp-buf, hpp-size, %6.2f%%,
percent); \
+
\
+ if (symbol_conf.event_group) {
\
+ int i;
\
+ struct perf_evsel *evsel = hists_to_evsel(hists);
\
+ struct hist_entry *pair;
\
+ int nr_members = evsel-nr_members;
\
+ double *percents;
\
+
\
+ if (nr_members = 1)
\
+ return ret;
\
+
\
+ percents = zalloc(sizeof(*percents) * nr_members);
\
+ if (percents == NULL) {
\
+ pr_warning(Not enough memory!\n);
\
+ return ret;
\
+ }
\
+
\
+ list_for_each_entry(pair, he-pairs.head, pairs.node) {
\
+ u64 period = pair-stat._field;
\
+ u64 total = pair-hists-stats.total_period;
\
+
\
+ if (!total)
\
+ continue;
\
+
\
+ evsel = hists_to_evsel(pair-hists);
\
+ i = perf_evsel__group_idx(evsel);
\
+ percents[i] = 100.0 * period / total;
\
+ }
\
+
\
+ for (i = 1; i nr_members; i++) {
\
+ ret += percent_color_snprintf(hpp-buf + ret,
\
+ hpp-size - ret,
\
+%6.2f%%,
percents[i]); \
+ }
\
+ free(percents);
\
+ }
\
+ return ret;
\
+}
ok, so this is the part thats common for both multi diff and group
report and hugely depends on how we link matching hist_entry
To sum to what group report does here:
1) starting with event group
2) the function 'he' belongs to leader hists
3) display leaders data value
4) if 'symbol_conf.event_group' is enabled, we want to display all
group members data values in a column
5) say we have group of 3 events 'e1,e2,e3' and e1 being the leader,
we have following possibilities:
- e1 have no pairs
- e1 is paired with e2
- e1 is paired with e3
- e1 is paired with e2 and e3 (e1 could also be dummy one)
6) need to display 3 values wrt to e1,e2,e3 output order
7) because events belongs to a group, each evsel carries group idx
8) so we walk all pairs and compute the eX value and put it
into temporary array based on its group idx
9)
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
Em Fri, Nov 30, 2012 at 02:29:43PM +0100, Jiri Olsa escreveu:
> On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
> > +#define __HPP_COLOR_PERCENT_FN(_type, _field)
> > \
> > +static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry *he)
> > \
> > +{
> > \
> > + int ret;
> > \
> > + double percent = 0.0;
> > \
> > + struct hists *hists = he->hists;
> > \
> > +
> > \
> > + if (hists->stats.total_period)
> > \
> > + percent = 100.0 * he->stat._field / hists->stats.total_period;
> > \
> > +
> > \
> > + ret = percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
> > \
> > +
> > \
> > + if (symbol_conf.event_group) {
> > \
> > + int i;
> > \
> > + struct perf_evsel *evsel = hists_to_evsel(hists);
> > \
> > + struct hist_entry *pair;
> > \
> > + int nr_members = evsel->nr_members;
> > \
> > + double *percents;
> > \
> > +
> > \
> > + if (nr_members <= 1)
> > \
> > + return ret;
> > \
> > +
> > \
> > + percents = zalloc(sizeof(*percents) * nr_members);
> > \
> > + if (percents == NULL) {
> > \
> > + pr_warning("Not enough memory!\n");
> > \
> > + return ret;
> > \
> > + }
> > \
Why do we need to zalloc this percents array?
> > + list_for_each_entry(pair, >pairs.head, pairs.node) {
> > \
> > + u64 period = pair->stat._field;
> > \
> > + u64 total = pair->hists->stats.total_period;
> > \
> > +
> > \
> > + if (!total)
> > \
> > + continue;
> > \
> > +
> > \
> > + evsel = hists_to_evsel(pair->hists);
> > \
> > + i = perf_evsel__group_idx(evsel);
> > \
> > + percents[i] = 100.0 * period / total;
> > \
Why not use percent_color_snprintf here, using some "%*s" thing that
uses i to position the output in the right column? This way the
following loop can be ditched as well. No?
> > + }
> > \
> > +
> > \
> > + for (i = 1; i < nr_members; i++) {
> > \
> > + ret += percent_color_snprintf(hpp->buf + ret,
> > \
> > + hpp->size - ret,
> > \
> > + " %6.2f%%", percents[i]);
> > \
> > + }
> > \
> > + free(percents);
> > \
> > + }
> > \
> > + return ret;
> > \
- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/18] perf ui/hist: Add support for event group view
Em Fri, Nov 30, 2012 at 02:29:43PM +0100, Jiri Olsa escreveu:
On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
+#define __HPP_COLOR_PERCENT_FN(_type, _field)
\
+static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry *he)
\
+{
\
+ int ret;
\
+ double percent = 0.0;
\
+ struct hists *hists = he-hists;
\
+
\
+ if (hists-stats.total_period)
\
+ percent = 100.0 * he-stat._field / hists-stats.total_period;
\
+
\
+ ret = percent_color_snprintf(hpp-buf, hpp-size, %6.2f%%, percent);
\
+
\
+ if (symbol_conf.event_group) {
\
+ int i;
\
+ struct perf_evsel *evsel = hists_to_evsel(hists);
\
+ struct hist_entry *pair;
\
+ int nr_members = evsel-nr_members;
\
+ double *percents;
\
+
\
+ if (nr_members = 1)
\
+ return ret;
\
+
\
+ percents = zalloc(sizeof(*percents) * nr_members);
\
+ if (percents == NULL) {
\
+ pr_warning(Not enough memory!\n);
\
+ return ret;
\
+ }
\
Why do we need to zalloc this percents array?
+ list_for_each_entry(pair, he-pairs.head, pairs.node) {
\
+ u64 period = pair-stat._field;
\
+ u64 total = pair-hists-stats.total_period;
\
+
\
+ if (!total)
\
+ continue;
\
+
\
+ evsel = hists_to_evsel(pair-hists);
\
+ i = perf_evsel__group_idx(evsel);
\
+ percents[i] = 100.0 * period / total;
\
Why not use percent_color_snprintf here, using some %*s thing that
uses i to position the output in the right column? This way the
following loop can be ditched as well. No?
+ }
\
+
\
+ for (i = 1; i nr_members; i++) {
\
+ ret += percent_color_snprintf(hpp-buf + ret,
\
+ hpp-size - ret,
\
+ %6.2f%%, percents[i]);
\
+ }
\
+ free(percents);
\
+ }
\
+ return ret;
\
- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 12/18] perf ui/hist: Add support for event group view
From: Namhyung Kim
Show group members' overhead also when showing the leader's if event
group is enabled. Use macro for defining hpp functions which looks
almost identical.
Cc: Jiri Olsa
Cc: Stephane Eranian
Signed-off-by: Namhyung Kim
---
tools/perf/ui/hist.c | 370 +++--
tools/perf/ui/stdio/hist.c | 2 +
2 files changed, 191 insertions(+), 181 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index aa84130024d5..b42bd15af3f5 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -3,152 +3,198 @@
#include "../util/hist.h"
#include "../util/util.h"
#include "../util/sort.h"
-
+#include "../util/evsel.h"
/* hist period print (hpp) functions */
-static int hpp__header_overhead(struct perf_hpp *hpp)
-{
- return scnprintf(hpp->buf, hpp->size, "Overhead");
-}
-
-static int hpp__width_overhead(struct perf_hpp *hpp __maybe_unused)
-{
- return 8;
-}
-
-static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period / hists->stats.total_period;
-
- return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
-}
-
-static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period / hists->stats.total_period;
- const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
-
- return scnprintf(hpp->buf, hpp->size, fmt, percent);
-}
-
-static int hpp__header_overhead_sys(struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
-
- return scnprintf(hpp->buf, hpp->size, fmt, "sys");
-}
-
-static int hpp__width_overhead_sys(struct perf_hpp *hpp __maybe_unused)
-{
- return 7;
-}
-
-static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period_sys /
hists->stats.total_period;
-
- return percent_color_snprintf(hpp->buf, hpp->size, "%6.2f%%", percent);
-}
-
-static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period_sys /
hists->stats.total_period;
- const char *fmt = symbol_conf.field_sep ? "%.2f" : "%6.2f%%";
-
- return scnprintf(hpp->buf, hpp->size, fmt, percent);
-}
-
-static int hpp__header_overhead_us(struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
-
- return scnprintf(hpp->buf, hpp->size, fmt, "user");
-}
-
-static int hpp__width_overhead_us(struct perf_hpp *hpp __maybe_unused)
-{
- return 7;
-}
-
-static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period_us / hists->stats.total_period;
-
- return percent_color_snprintf(hpp->buf, hpp->size, "%6.2f%%", percent);
-}
-
-static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period_us / hists->stats.total_period;
- const char *fmt = symbol_conf.field_sep ? "%.2f" : "%6.2f%%";
-
- return scnprintf(hpp->buf, hpp->size, fmt, percent);
-}
-
-static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)
-{
- return scnprintf(hpp->buf, hpp->size, "guest sys");
-}
-
-static int hpp__width_overhead_guest_sys(struct perf_hpp *hpp __maybe_unused)
-{
- return 9;
-}
-
-static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
-struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period_guest_sys /
hists->stats.total_period;
-
- return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%% ",
percent);
-}
-
-static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
-struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period_guest_sys /
hists->stats.total_period;
- const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%% ";
-
- return scnprintf(hpp->buf, hpp->size, fmt, percent);
-}
-
-static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)
-{
- return scnprintf(hpp->buf, hpp->size, "guest usr");
-}
-
-static int hpp__width_overhead_guest_us(struct perf_hpp *hpp __maybe_unused)
-{
- return 9;
-}
-
-static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
- struct hist_entry *he)
-{
- struct hists *hists = he->hists;
- double percent = 100.0 * he->stat.period_guest_us /
hists->stats.total_period;
-
- return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%
[PATCH 12/18] perf ui/hist: Add support for event group view
From: Namhyung Kim namhyung@lge.com
Show group members' overhead also when showing the leader's if event
group is enabled. Use macro for defining hpp functions which looks
almost identical.
Cc: Jiri Olsa jo...@redhat.com
Cc: Stephane Eranian eran...@google.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
tools/perf/ui/hist.c | 370 +++--
tools/perf/ui/stdio/hist.c | 2 +
2 files changed, 191 insertions(+), 181 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index aa84130024d5..b42bd15af3f5 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -3,152 +3,198 @@
#include ../util/hist.h
#include ../util/util.h
#include ../util/sort.h
-
+#include ../util/evsel.h
/* hist period print (hpp) functions */
-static int hpp__header_overhead(struct perf_hpp *hpp)
-{
- return scnprintf(hpp-buf, hpp-size, Overhead);
-}
-
-static int hpp__width_overhead(struct perf_hpp *hpp __maybe_unused)
-{
- return 8;
-}
-
-static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period / hists-stats.total_period;
-
- return percent_color_snprintf(hpp-buf, hpp-size, %6.2f%%, percent);
-}
-
-static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period / hists-stats.total_period;
- const char *fmt = symbol_conf.field_sep ? %.2f : %6.2f%%;
-
- return scnprintf(hpp-buf, hpp-size, fmt, percent);
-}
-
-static int hpp__header_overhead_sys(struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? %s : %7s;
-
- return scnprintf(hpp-buf, hpp-size, fmt, sys);
-}
-
-static int hpp__width_overhead_sys(struct perf_hpp *hpp __maybe_unused)
-{
- return 7;
-}
-
-static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period_sys /
hists-stats.total_period;
-
- return percent_color_snprintf(hpp-buf, hpp-size, %6.2f%%, percent);
-}
-
-static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period_sys /
hists-stats.total_period;
- const char *fmt = symbol_conf.field_sep ? %.2f : %6.2f%%;
-
- return scnprintf(hpp-buf, hpp-size, fmt, percent);
-}
-
-static int hpp__header_overhead_us(struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? %s : %7s;
-
- return scnprintf(hpp-buf, hpp-size, fmt, user);
-}
-
-static int hpp__width_overhead_us(struct perf_hpp *hpp __maybe_unused)
-{
- return 7;
-}
-
-static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period_us / hists-stats.total_period;
-
- return percent_color_snprintf(hpp-buf, hpp-size, %6.2f%%, percent);
-}
-
-static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period_us / hists-stats.total_period;
- const char *fmt = symbol_conf.field_sep ? %.2f : %6.2f%%;
-
- return scnprintf(hpp-buf, hpp-size, fmt, percent);
-}
-
-static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)
-{
- return scnprintf(hpp-buf, hpp-size, guest sys);
-}
-
-static int hpp__width_overhead_guest_sys(struct perf_hpp *hpp __maybe_unused)
-{
- return 9;
-}
-
-static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
-struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period_guest_sys /
hists-stats.total_period;
-
- return percent_color_snprintf(hpp-buf, hpp-size, %6.2f%% ,
percent);
-}
-
-static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
-struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period_guest_sys /
hists-stats.total_period;
- const char *fmt = symbol_conf.field_sep ? %.2f : %6.2f%% ;
-
- return scnprintf(hpp-buf, hpp-size, fmt, percent);
-}
-
-static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)
-{
- return scnprintf(hpp-buf, hpp-size, guest usr);
-}
-
-static int hpp__width_overhead_guest_us(struct perf_hpp *hpp __maybe_unused)
-{
- return 9;
-}
-
-static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
- struct hist_entry *he)
-{
- struct hists *hists = he-hists;
- double percent = 100.0 * he-stat.period_guest_us /
hists-stats.total_period;
-
- return percent_color_snprintf(hpp-buf, hpp-size, %6.2f%% ,
percent);
-}
-
-static int
