Messages in this thread | | | Date | Fri, 6 Oct 2017 15:54:22 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v1 1/4] perf annotate: create a new hists to manage multiple events samples |
| |
Em Sat, Oct 07, 2017 at 12:31:37AM +0800, Jin, Yao escreveu: > On 10/5/2017 9:21 PM, Arnaldo Carvalho de Melo wrote: > > 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. > > > > Right, for --group we know that we'll be reading all the counters at > > each sample, so it all works and we can use the current design.
> > When we're not using groups tho, each sample has just one of the events > > and we end up with separate views.
> > Note tho that since the annotation buckets are kept per 'struct symbol' > > instance, this problem should be present only in the hist_entry based > > views, i.e. 'perf report' and 'perf top', right?
> Yes, it seems to be in hist_entry based view.
Ok.
But note that your initial statement of the problem:
<quote Ji, Yao> 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. </>
Can be solved right now, its just a matter of accessing the other buckets in a given symbol, just like we have for --group.
The only problem is in presenting a list of symbols which can be annotated, we have them for each evsel, and you, rightly, want to show the list of all symbols in all evsels.
Ok?
> > I.e. all struct hist_entry->ms.sym instances point to the same stuct > > symbol and thus will use the same annotation histogram buckets, i.e. > > symbol__annotation(hist_entry->ms.sym) point to the same 'struct > > annotation' instance, and then its a matter of passing this pointer to > > annotation__histogram(notes, IDX) where this IDX is perf_evsel->idx.
> > I wonder if we can't just add a new rb_node in struct hist_entry and > > have it in two rb_trees, i.e. in two 'struct hists' instances, one per > > evsel and one per evlist.
> Currently we just have per-evsel hists. This idea will create a new per-evlist hists.
> struct perf_evlist { > ...... > struct hists hists; > };
> And let the hist_entry be linked in both per-evsel hists and per-evlist hists.
> Please correct me if my understanding is wrong for this idea.
Yes, do you see problems with trying to do it this way? At a first sight seems like it will reuse more code, no?
I.e. in hists__findnew_entry(), when not finding an existing hist_entry in the per-evsel hists you end up calling hist_entry__new(), right here you'll add it to the evsel->evlist->hists, and when we want to go from a hist_entry to the evlist it is in we'll use:
hists_to_evsel(he->hists)->evlist
Right?
- Arnaldo
| |