Em Wed, Aug 16, 2017 at 06:18:33PM +0800, Jin Yao escreveu:
> An issue is found during using perf annotate.
>
> perf record -e cycles,branches ...
> perf annotate main --stdio
>
> The result only shows cycles. It should show both cycles and
> branches on the left side. It works with "--group", but need
> this to work even without groups.
>
> In current design, the hists is per event. So we need a new
> hists to manage the samples for multiple events and use a new
> hist_event data structure to save the map/symbol information
> for per event.
Humm, why do we need another hists? Don't we have one per evsel, don't
we have a evlist from where to get all of those evsels, can't we just
use that to add one column per evsel?
- Arnaldo
> Signed-off-by: Jin Yao <yao....@linux.intel.com>
> ---
> tools/perf/builtin-annotate.c | 60
> +++++++++++++++++++++++++++++++------------
> tools/perf/util/annotate.c | 8 ++++++
> tools/perf/util/annotate.h | 4 +++
> tools/perf/util/sort.c | 21 +++++++++++++++
> tools/perf/util/sort.h | 13 ++++++++++
> 5 files changed, 89 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 658c920..833866c 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -45,6 +45,7 @@ struct perf_annotate {
> bool skip_missing;
> const char *sym_hist_filter;
> const char *cpu_list;
> + struct hists events_hists;
> DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
> };
>
> @@ -153,6 +154,7 @@ static int perf_evsel__add_sample(struct perf_evsel
> *evsel,
> struct hists *hists = evsel__hists(evsel);
> struct hist_entry *he;
> int ret;
> + struct hist_event *hevt;
>
> if (ann->sym_hist_filter != NULL &&
> (al->sym == NULL ||
> @@ -177,12 +179,30 @@ static int perf_evsel__add_sample(struct perf_evsel
> *evsel,
> */
> process_branch_stack(sample->branch_stack, al, sample);
>
> - he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
> - if (he == NULL)
> - return -ENOMEM;
> + if (symbol_conf.nr_events == 1) {
> + he = hists__add_entry(hists, al, NULL, NULL, NULL,
> + sample, true);
> + if (he == NULL)
> + return -ENOMEM;
> +
> + ret = hist_entry__inc_addr_samples(he, sample, evsel->idx,
> + al->addr);
> + hists__inc_nr_samples(hists, true);
> + } else {
> + he = hists__add_entry(&ann->events_hists, al, NULL,
> + NULL, NULL, sample, true);
> + if (he == NULL)
> + return -ENOMEM;
> +
> + hevt = hist_entry__event_add(he, evsel);
> + if (hevt == NULL)
> + return -ENOMEM;
> +
> + ret = hist_event__inc_addr_samples(hevt, sample, hevt->idx,
> + al->addr);
> + hists__inc_nr_samples(&ann->events_hists, true);
> + }
>
> - ret = hist_entry__inc_addr_samples(he, sample, evsel->idx, al->addr);
> - hists__inc_nr_samples(hists, true);
> return ret;
> }
>
> @@ -304,7 +324,8 @@ static int __cmd_annotate(struct perf_annotate *ann)
> int ret;
> struct perf_session *session = ann->session;
> struct perf_evsel *pos;
> - u64 total_nr_samples;
> + u64 total_nr_samples = 0;
> + struct hists *hists;
>
> if (ann->cpu_list) {
> ret = perf_session__cpu_bitmap(session, ann->cpu_list,
> @@ -335,23 +356,26 @@ static int __cmd_annotate(struct perf_annotate *ann)
> if (verbose > 2)
> perf_session__fprintf_dsos(session, stdout);
>
> - total_nr_samples = 0;
> - evlist__for_each_entry(session->evlist, pos) {
> - struct hists *hists = evsel__hists(pos);
> - u32 nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
> + if (symbol_conf.nr_events > 1) {
> + hists = &ann->events_hists;
> + total_nr_samples +=
> + hists->stats.nr_events[PERF_RECORD_SAMPLE];
> +
> + hists__collapse_resort(hists, NULL);
> + hists__output_resort(hists, NULL);
> + hists__find_annotations(hists, NULL, ann);
> + } else {
> + evlist__for_each_entry(session->evlist, pos) {
> + hists = evsel__hists(pos);
> + total_nr_samples +=
> + hists->stats.nr_events[PERF_RECORD_SAMPLE];
>
> - if (nr_samples > 0) {
> - total_nr_samples += nr_samples;
> hists__collapse_resort(hists, NULL);
> /* Don't sort callchain */
> perf_evsel__reset_sample_bit(pos, CALLCHAIN);
> perf_evsel__output_resort(pos, NULL);
> -
> - if (symbol_conf.event_group &&
> - !perf_evsel__is_group_leader(pos))
> - continue;
> -
> hists__find_annotations(hists, pos, ann);
> + break;
> }
> }
>
> @@ -486,6 +510,8 @@ int cmd_annotate(int argc, const char **argv)
> if (ret < 0)
> goto out_delete;
>
> + __hists__init(&annotate.events_hists, &perf_hpp_list);
> +
> if (setup_sorting(NULL) < 0)
> usage_with_options(annotate_usage, options);
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 2dab0e5..16ec881 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -828,6 +828,14 @@ int hist_entry__inc_addr_samples(struct hist_entry *he,
> struct perf_sample *samp
> return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip,
> sample);
> }
>
> +int hist_event__inc_addr_samples(struct hist_event *hevt,
> + struct perf_sample *sample,
> + int idx, u64 ip)
> +{
> + return symbol__inc_addr_samples(hevt->ms.sym, hevt->ms.map,
> + idx, ip, sample);
> +}
> +
> static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch,
> struct map *map)
> {
> dl->ins.ops = ins__find(arch, dl->ins.name);
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 9ce575c..0d44cfe 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -165,6 +165,10 @@ int addr_map_symbol__account_cycles(struct
> addr_map_symbol *ams,
> int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample
> *sample,
> int evidx, u64 addr);
>
> +int hist_event__inc_addr_samples(struct hist_event *hevt,
> + struct perf_sample *sample,
> + int idx, u64 addr);
> +
> int symbol__alloc_hist(struct symbol *sym);
> void symbol__annotate_zero_histograms(struct symbol *sym);
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 12359bd..1500a8e 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1490,6 +1490,27 @@ struct sort_entry sort_sym_size = {
> .se_width_idx = HISTC_SYM_SIZE,
> };
>
> +struct hist_event *hist_entry__event_add(struct hist_entry *he,
> + struct perf_evsel *evsel)
> +{
> + int i;
> +
> + for (i = 0; i < he->event_nr; i++) {
> + if (he->events[i].evsel == evsel)
> + return &he->events[i];
> + }
> +
> + if (i < HIST_ENTRY_EVENTS) {
> + memset(&he->events[i], 0, sizeof(struct hist_event));
> + memcpy(&he->events[i].ms, &he->ms, sizeof(struct map_symbol));
> + he->events[i].evsel = evsel;
> + he->events[i].idx = i;
> + he->event_nr++;
> + return &he->events[i];
> + }
> +
> + return NULL;
> +}
>
> struct sort_dimension {
> const char *name;
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index b7c7559..446a2a3 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -79,6 +79,14 @@ struct hist_entry_ops {
> void (*free)(void *ptr);
> };
>
> +#define HIST_ENTRY_EVENTS 8
> +
> +struct hist_event {
> + struct map_symbol ms;
> + struct perf_evsel *evsel;
> + int idx;
> +};
> +
> /**
> * struct hist_entry - histogram entry
> *
> @@ -95,6 +103,8 @@ struct hist_entry {
> struct he_stat stat;
> struct he_stat *stat_acc;
> struct map_symbol ms;
> + struct hist_event events[HIST_ENTRY_EVENTS];
> + int event_nr;
> struct thread *thread;
> struct comm *comm;
> struct namespace_id cgroup_id;
> @@ -291,4 +301,7 @@ sort__daddr_cmp(struct hist_entry *left, struct
> hist_entry *right);
> int64_t
> sort__dcacheline_cmp(struct hist_entry *left, struct hist_entry *right);
> char *hist_entry__get_srcline(struct hist_entry *he);
> +struct hist_event *hist_entry__event_add(struct hist_entry *he,
> + struct perf_evsel *evsel);
> +
> #endif /* __PERF_SORT_H */
> --
> 2.7.4
