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 <[email protected]>
> ---
> 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
