Currently data_type_cmp() only compares size and type name.
But in cases where the type name of two data type entries
is same, but var_name is different, the comparison can't distinguish
two different types.

Consider there is a "long unsigned int" with var_name as "X" and there
is global variable "long unsigned int". Currently since
data_type_cmp uses only type_name for comparison ( "long unsigned int"),
it won't distinguish these as separate entries. Update the
functions "data_type_cmp" as well as "sort__typeoff_sort" to
compare variable names after type name if it exists. Inorder to
use cmp_null, make the cmp_null from sort.c as not static.

Also updated "hist_entry__typeoff_snprintf" to print var_name if
it is set. With the changes,

     11.42%  long unsigned int  long unsigned int +0 (current_stack_pointer)
     4.68%  struct paca_struct  struct paca_struct +2312 (__current)
     4.57%  struct paca_struct  struct paca_struct +2354 (irq_soft_mask)
     2.69%  struct paca_struct  struct paca_struct +2808 (canary)
     2.68%  struct paca_struct  struct paca_struct +8 (paca_index)
     2.24%  struct paca_struct  struct paca_struct +48 (data_offset)
     1.43%  long unsigned int  long unsigned int +0 (no field)

Using ./perf report -s type,typeoff -H:

    17.65%     struct paca_struct
        4.68%     struct paca_struct +2312 (__current)
        4.57%     struct paca_struct +2354 (irq_soft_mask)
        2.69%     struct paca_struct +2808 (canary)
        2.68%     struct paca_struct +8 (paca_index)
        2.24%     struct paca_struct +48 (data_offset)
        0.55%     struct paca_struct +2816 (mmiowb_state.nesting_count)
        0.18%     struct paca_struct +2818 (mmiowb_state.mmiowb_pending)
        0.03%     struct paca_struct +2352 (hsrr_valid)
        0.02%     struct paca_struct +2356 (irq_work_pending)
        0.00%     struct paca_struct +0 (lppaca_ptr)
    12.85%     long unsigned int
       11.42%     long unsigned int +0 (current_stack_pointer)
        1.43%     long unsigned int +0 (no field)

With perf report -s type:

     17.65%  struct paca_struct
     12.85%  long unsigned int
      1.69%  struct task_struct
      1.51%  struct rq

with perf report -s typeoff

    11.42%  long unsigned int +0 (current_stack_pointer)
     4.68%  struct paca_struct +2312 (__current)
     4.57%  struct paca_struct +2354 (irq_soft_mask)
     2.69%  struct paca_struct +2808 (canary)
     2.68%  struct paca_struct +8 (paca_index)
     2.24%  struct paca_struct +48 (data_offset)
     1.43%  long unsigned int +0 (no field)

Signed-off-by: Athira Rajeev <atraj...@linux.vnet.ibm.com>
---
 tools/perf/util/annotate-data.c | 23 +++++++++++++++++++++--
 tools/perf/util/sort.c          | 25 ++++++++++++++++++++++---
 tools/perf/util/sort.h          |  1 +
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 8d05f3dbddf6..ea69c8d3d856 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -167,7 +167,7 @@ static void exit_type_state(struct type_state *state)
 }
 
 /*
- * Compare type name and size to maintain them in a tree.
+ * Compare type name, var_name  and size to maintain them in a tree.
  * I'm not sure if DWARF would have information of a single type in many
  * different places (compilation units).  If not, it could compare the
  * offset of the type entry in the .debug_info section.
@@ -176,12 +176,31 @@ static int data_type_cmp(const void *_key, const struct 
rb_node *node)
 {
        const struct annotated_data_type *key = _key;
        struct annotated_data_type *type;
+       int64_t ret = 0;
 
        type = rb_entry(node, struct annotated_data_type, node);
 
        if (key->self.size != type->self.size)
                return key->self.size - type->self.size;
-       return strcmp(key->self.type_name, type->self.type_name);
+
+       ret = strcmp(key->self.type_name, type->self.type_name);
+       if (ret)
+               return ret;
+
+       /*
+        * Compare var_name if it exists for key and type.
+        * If both nodes doesn't have var_name, but one of
+        * them has, return non-zero. This is to indicate nodes
+        * are not the same if one has var_name, but other doesn't.
+        */
+       if (key->self.var_name && type->self.var_name) {
+               ret = strcmp(key->self.var_name, type->self.var_name);
+               if (ret)
+                       return ret;
+       } else if (!key->self.var_name != !type->self.var_name)
+               return cmp_null(key->self.var_name, type->self.var_name);
+
+       return ret;
 }
 
 static bool data_type_less(struct rb_node *node_a, const struct rb_node 
*node_b)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index cd39ea972193..25761d01dbd0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -95,7 +95,7 @@ static int repsep_snprintf(char *bf, size_t size, const char 
*fmt, ...)
        return n;
 }
 
-static int64_t cmp_null(const void *l, const void *r)
+int64_t cmp_null(const void *l, const void *r)
 {
        if (!l && !r)
                return 0;
@@ -2267,9 +2267,25 @@ sort__typeoff_sort(struct hist_entry *left, struct 
hist_entry *right)
                right_type = right->mem_type;
        }
 
+       /*
+        * Compare type_name first. Next, ompare var_name if it exists
+        * for left and right hist_entry. If both entries doesn't have
+        * var_name, but one of them has, return non-zero. This is to
+        * indicate entries are not the same if one has var_name, but the
+        * other doesn't.
+        * If type_name and var_name is same, use mem_type_off field.
+        */
        ret = strcmp(left_type->self.type_name, right_type->self.type_name);
        if (ret)
                return ret;
+
+       if (left_type->self.var_name && right_type->self.var_name) {
+               ret = strcmp(left_type->self.var_name, 
right_type->self.var_name);
+               if (ret)
+                       return ret;
+       } else if (!left_type->self.var_name != !right_type->self.var_name)
+               return cmp_null(left_type->self.var_name, 
right_type->self.var_name);
+
        return left->mem_type_off - right->mem_type_off;
 }
 
@@ -2305,9 +2321,12 @@ static int hist_entry__typeoff_snprintf(struct 
hist_entry *he, char *bf,
        char buf[4096];
 
        buf[0] = '\0';
-       if (list_empty(&he_type->self.children))
+       if (list_empty(&he_type->self.children)) {
                snprintf(buf, sizeof(buf), "no field");
-       else
+               if (he_type->self.var_name)
+                       strcpy(buf, he_type->self.var_name);
+
+       } else
                fill_member_name(buf, sizeof(buf), &he_type->self,
                                 he->mem_type_off, true);
        buf[4095] = '\0';
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 0bd0ee3ae76b..41346d2b940e 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -151,4 +151,5 @@ sort__dcacheline_cmp(struct hist_entry *left, struct 
hist_entry *right);
 int64_t
 _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r);
 char *hist_entry__srcline(struct hist_entry *he);
+int64_t cmp_null(const void *l, const void *r);
 #endif /* __PERF_SORT_H */
-- 
2.43.0

Reply via email to