Em Sat, Feb 27, 2016 at 03:52:47AM +0900, Namhyung Kim escreveu: > The column width of dynamic entries is updated when comparing hist > entries. However some unique entries can miss the chance to update. So > move the update to output resort stage to make sure every entry will get > called before display. > > To do that, abuse ->sort callback to update the width when the third > argument is NULL. When resorting entries in normal path, it never be > NULL so it should be fine IMHO. > > Before: > > # Overhead ptr / bytes_req / gfp_flags > # .............. .......................................... > # > 37.50% 0xffff8803f7669400 > 37.50% 448 > 37.50% GFP_ATOMIC|GFP_NOWARN|GFP_NOMEMALLOC > 10.42% 0xffff8803f766be00 > 8.33% 96 > 8.33% GFP_ATOMIC|GFP_NOWARN|GFP_NOMEMALLOC > 2.08% 512 > 2.08% GFP_KERNEL|GFP_NOWARN|GFP_REPEAT|GFP <-- here
So, I tested this with: perf report ... > /tmp/before apply the patch, and then do > /tmp/after, do the diff, reproduced what you described, slightly different output: [root@felicio ~]# diff -u /tmp/before /tmp/after | grep ^[+-] --- /tmp/before 2016-02-26 19:38:31.321497576 -0300 +++ /tmp/after 2016-02-26 19:39:24.043614934 -0300 -# .............. .................................... +# .............. ........................................ - 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA + 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC - 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA + 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC - 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA + 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC - 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA + 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC - 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA + 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC - 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA + 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC - 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMA + 0.35% GFP_ATOMIC|GFP_ZERO|GFP_NOMEMALLOC [root@felicio ~]# diff -u /tmp/before /tmp/after | grep ^[+-] This is something else to add to the regression tests, output diff for as many command line combos outputs as we can... Anyway, good work, all applied with Jiri's acks. - Arnaldo > > After: > > # Overhead ptr / bytes_req / gfp_flags > # .............. ..................................................... > # > 37.50% 0xffff8803f7669400 > 37.50% 448 > 37.50% GFP_ATOMIC|GFP_NOWARN|GFP_NOMEMALLOC > 10.42% 0xffff8803f766be00 > 8.33% 96 > 8.33% GFP_ATOMIC|GFP_NOWARN|GFP_NOMEMALLOC > 2.08% 512 > 2.08% GFP_KERNEL|GFP_NOWARN|GFP_REPEAT|GFP_NOMEMALLOC > > Signed-off-by: Namhyung Kim <[email protected]> > --- > tools/perf/util/hist.c | 11 +++++++++++ > tools/perf/util/sort.c | 8 +++++--- > 2 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index 9b3f582867d6..4b8b67bc0cd8 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -1371,6 +1371,10 @@ static void hierarchy_insert_output_entry(struct > rb_root *root, > > rb_link_node(&he->rb_node, parent, p); > rb_insert_color(&he->rb_node, root); > + > + /* update column width of dynamic entry */ > + if (perf_hpp__is_dynamic_entry(he->fmt)) > + he->fmt->sort(he->fmt, he, NULL); > } > > static void hists__hierarchy_output_resort(struct hists *hists, > @@ -1440,6 +1444,7 @@ static void __hists__insert_output_entry(struct rb_root > *entries, > struct rb_node **p = &entries->rb_node; > struct rb_node *parent = NULL; > struct hist_entry *iter; > + struct perf_hpp_fmt *fmt; > > if (use_callchain) { > if (callchain_param.mode == CHAIN_GRAPH_REL) { > @@ -1466,6 +1471,12 @@ static void __hists__insert_output_entry(struct > rb_root *entries, > > rb_link_node(&he->rb_node, parent, p); > rb_insert_color(&he->rb_node, entries); > + > + perf_hpp_list__for_each_sort_list(&perf_hpp_list, fmt) { > + if (perf_hpp__is_dynamic_entry(fmt) && > + perf_hpp__defined_dynamic_entry(fmt, he->hists)) > + fmt->sort(fmt, he, NULL); /* update column width */ > + } > } > > static void output_resort(struct hists *hists, struct ui_progress *prog, > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > index 3b1b4018f111..1457c9d975a0 100644 > --- a/tools/perf/util/sort.c > +++ b/tools/perf/util/sort.c > @@ -1819,6 +1819,11 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt > *fmt, > > hde = container_of(fmt, struct hpp_dynamic_entry, hpp); > > + if (b == NULL) { > + update_dynamic_len(hde, a); > + return 0; > + } > + > field = hde->field; > if (field->flags & FIELD_IS_DYNAMIC) { > unsigned long long dyn; > @@ -1833,9 +1838,6 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt, > } else { > offset = field->offset; > size = field->size; > - > - update_dynamic_len(hde, a); > - update_dynamic_len(hde, b); > } > > return memcmp(a->raw_data + offset, b->raw_data + offset, size); > -- > 2.7.1
