Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1344581 > unrolled thread
| Started by | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| First post | 2016-02-26 20:00 +0100 |
| Last post | 2016-02-27 10:50 +0100 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 5/5] perf report: Update column width of dynamic entries Namhyung Kim <namhyung@kernel.org> - 2016-02-26 20:00 +0100
Re: [PATCH 5/5] perf report: Update column width of dynamic entries Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-02-26 23:50 +0100
[tip:perf/core] perf report: Update column width of dynamic entries tip-bot for Namhyung Kim <tipbot@zytor.com> - 2016-02-27 10:50 +0100
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-02-26 20:00 +0100 |
| Subject | [PATCH 5/5] perf report: Update column width of dynamic entries |
| Message-ID | <r6vMm-7w1-11@gated-at.bofh.it> |
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
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 <namhyung@kernel.org>
---
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
[toc] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-02-26 23:50 +0100 |
| Message-ID | <r6zmW-1EG-11@gated-at.bofh.it> |
| In reply to | #1344581 |
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 <namhyung@kernel.org>
> ---
> 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
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Namhyung Kim <tipbot@zytor.com> |
|---|---|
| Date | 2016-02-27 10:50 +0100 |
| Subject | [tip:perf/core] perf report: Update column width of dynamic entries |
| Message-ID | <r6JFE-XE-17@gated-at.bofh.it> |
| In reply to | #1344581 |
Commit-ID: abab5e7fcec16e526968f8a5448cd81c635705ce
Gitweb: http://git.kernel.org/tip/abab5e7fcec16e526968f8a5448cd81c635705ce
Author: Namhyung Kim <namhyung@kernel.org>
AuthorDate: Sat, 27 Feb 2016 03:52:47 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 26 Feb 2016 19:38:48 -0300
perf report: Update column width of dynamic entries
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
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 <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1456512767-1164-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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 9b3f582..4b8b67b 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 d26c6b9..5888bfe 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1816,6 +1816,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;
@@ -1830,9 +1835,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);
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web