Messages in this thread Patch in this message | | | From | Arnaldo Carvalho de Melo <> | Date | Tue, 2 Jul 2019 13:17:39 -0300 | Subject | Re: [PATCH v6 5/7] perf diff: Link same basic blocks among different data |
| |
Em Fri, Jun 28, 2019 at 05:23:02PM +0800, Jin Yao escreveu: > The target is to compare the performance difference (cycles > diff) for the same basic blocks in different data files. > > The same basic block means same function, same start address > and same end address. This patch finds the same basic blocks > from different data files and link them together and resort > by the cycles diff. > > v3: > --- > The block stuffs are maintained by new structure 'block_hist', > so this patch is update accordingly. > > v2: > --- > Since now the basic block hists is changed to per symbol, > the patch only links the basic block hists for the same > symbol in different data files. > > Signed-off-by: Jin Yao <yao.jin@linux.intel.com> > --- > tools/perf/builtin-diff.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 90 insertions(+) > > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c > index 83b8c0f..823f162 100644 > --- a/tools/perf/builtin-diff.c > +++ b/tools/perf/builtin-diff.c > @@ -641,6 +641,85 @@ static int process_block_per_sym(struct hist_entry *he) > return 0; > } > > +static int block_pair_cmp(struct hist_entry *a, struct hist_entry *b) > +{ > + struct block_info *bi_a = a->block_info; > + struct block_info *bi_b = b->block_info; > + int cmp; > + > + if (!bi_a->sym || !bi_b->sym) > + return -1; > + > + if (bi_a->sym->name && bi_b->sym->name) { > + cmp = strcmp(bi_a->sym->name, bi_b->sym->name); > + if ((!cmp) && (bi_a->start == bi_b->start) && > + (bi_a->end == bi_b->end)) { > + return 0; > + }
builtin-diff.c:658:17: error: address of array 'bi_a->sym->name' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] if (bi_a->sym->name && bi_b->sym->name) { ~~~~~~~~~~~^~~~ ~~ builtin-diff.c:658:36: error: address of array 'bi_b->sym->name' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] if (bi_a->sym->name && bi_b->sym->name) {
Because:
struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *name) { size_t namelen = strlen(name) + 1; struct symbol *sym = calloc(1, (symbol_conf.priv_size + sizeof(*sym) + namelen));
So it will be at least a strlen(sym->name) == 0, i.e. we can use it without checking anything.
I'm chanign it to do the cmp straight away
> + } > + > + return -1; > +} > + > +static struct hist_entry *get_block_pair(struct hist_entry *he, > + struct hists *hists_pair) > +{ > + struct rb_root_cached *root = hists_pair->entries_in; > + struct rb_node *next = rb_first_cached(root); > + int cmp; > + > + while (next != NULL) { > + struct hist_entry *he_pair = rb_entry(next, struct hist_entry, > + rb_node_in); > + > + next = rb_next(&he_pair->rb_node_in); > + > + cmp = block_pair_cmp(he_pair, he); > + if (!cmp) > + return he_pair; > + } > + > + return NULL; > +} > + > +static void compute_cycles_diff(struct hist_entry *he, > + struct hist_entry *pair) > +{ > + pair->diff.computed = true; > + if (pair->block_info->num && he->block_info->num) { > + pair->diff.cycles = > + pair->block_info->cycles_aggr / pair->block_info->num_aggr - > + he->block_info->cycles_aggr / he->block_info->num_aggr; > + } > +} > + > +static void block_hists_match(struct hists *hists_base, > + struct hists *hists_pair) > +{ > + struct rb_root_cached *root = hists_base->entries_in; > + struct rb_node *next = rb_first_cached(root); > + > + while (next != NULL) { > + struct hist_entry *he = rb_entry(next, struct hist_entry, > + rb_node_in); > + struct hist_entry *pair = get_block_pair(he, hists_pair); > + > + next = rb_next(&he->rb_node_in); > + > + if (pair) { > + hist_entry__add_pair(pair, he); > + compute_cycles_diff(he, pair); > + } > + } > +} > + > +static int filter_cb(struct hist_entry *he, void *arg __maybe_unused) > +{ > + /* Skip the calculation of column length in output_resort */ > + he->filtered = true; > + return 0; > +} > + > static void hists__precompute(struct hists *hists) > { > struct rb_root_cached *root; > @@ -653,6 +732,7 @@ static void hists__precompute(struct hists *hists) > > next = rb_first_cached(root); > while (next != NULL) { > + struct block_hist *bh, *pair_bh; > struct hist_entry *he, *pair; > struct data__file *d; > int i; > @@ -681,6 +761,16 @@ static void hists__precompute(struct hists *hists) > break; > case COMPUTE_CYCLES: > process_block_per_sym(pair); > + bh = container_of(he, struct block_hist, he); > + pair_bh = container_of(pair, struct block_hist, > + he); > + > + if (bh->valid && pair_bh->valid) { > + block_hists_match(&bh->block_hists, > + &pair_bh->block_hists); > + hists__output_resort_cb(&pair_bh->block_hists, > + NULL, filter_cb); > + } > break; > default: > BUG_ON(1); > -- > 2.7.4
--
- Arnaldo
| |