* [PATCH 1/8] perf callchain: Convert children list to rbtree
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
@ 2013-09-26 8:58 ` Namhyung Kim
2013-10-02 10:18 ` Frederic Weisbecker
2013-09-26 8:58 ` [PATCH 2/8] perf ui/progress: Add new helper functions for progress bar Namhyung Kim
` (8 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Namhyung Kim @ 2013-09-26 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Linus Torvalds, Frederic Weisbecker, Jiri Olsa
From: Namhyung Kim <namhyung.kim@lge.com>
Current collapse stage has a scalability problem which can be
reproduced easily with parallel kernel build. This is because it
needs to traverse every children of callchain linearly during the
collapse/merge stage. Convert it to rbtree reduced the overhead
significantly.
On my 400MB perf.data file which recorded with make -j32 kernel build:
$ time perf --no-pager report --stdio > /dev/null
before:
real 6m22.073s
user 6m18.683s
sys 0m0.706s
after:
real 0m20.780s
user 0m19.962s
sys 0m0.689s
During the perf report the overhead on append_chain_children went down
from 96.69% to 18.16%:
- 18.16% perf perf [.] append_chain_children
- append_chain_children
- 77.48% append_chain_children
+ 69.79% merge_chain_branch
- 22.96% append_chain_children
+ 67.44% merge_chain_branch
+ 30.15% append_chain_children
+ 2.41% callchain_append
+ 7.25% callchain_append
+ 12.26% callchain_append
+ 10.22% merge_chain_branch
+ 11.58% perf perf [.] dso__find_symbol
+ 8.02% perf perf [.] sort__comm_cmp
+ 5.48% perf libc-2.17.so [.] malloc_consolidate
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Link: http://lkml.kernel.org/n/tip-d9tcfow6stbrp4btvgs51y67@git.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/callchain.c | 147 +++++++++++++++++++++++++++++++++-----------
tools/perf/util/callchain.h | 11 ++--
2 files changed, 116 insertions(+), 42 deletions(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 482f68081cd8..e3970e3eaacf 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -21,12 +21,6 @@
__thread struct callchain_cursor callchain_cursor;
-#define chain_for_each_child(child, parent) \
- list_for_each_entry(child, &parent->children, siblings)
-
-#define chain_for_each_child_safe(child, next, parent) \
- list_for_each_entry_safe(child, next, &parent->children, siblings)
-
static void
rb_insert_callchain(struct rb_root *root, struct callchain_node *chain,
enum chain_mode mode)
@@ -71,10 +65,16 @@ static void
__sort_chain_flat(struct rb_root *rb_root, struct callchain_node *node,
u64 min_hit)
{
+ struct rb_node *n;
struct callchain_node *child;
- chain_for_each_child(child, node)
+ n = rb_first(&node->rb_root_in);
+ while (n) {
+ child = rb_entry(n, struct callchain_node, rb_node_in);
+ n = rb_next(n);
+
__sort_chain_flat(rb_root, child, min_hit);
+ }
if (node->hit && node->hit >= min_hit)
rb_insert_callchain(rb_root, node, CHAIN_FLAT);
@@ -94,11 +94,16 @@ sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
static void __sort_chain_graph_abs(struct callchain_node *node,
u64 min_hit)
{
+ struct rb_node *n;
struct callchain_node *child;
node->rb_root = RB_ROOT;
+ n = rb_first(&node->rb_root_in);
+
+ while (n) {
+ child = rb_entry(n, struct callchain_node, rb_node_in);
+ n = rb_next(n);
- chain_for_each_child(child, node) {
__sort_chain_graph_abs(child, min_hit);
if (callchain_cumul_hits(child) >= min_hit)
rb_insert_callchain(&node->rb_root, child,
@@ -117,13 +122,18 @@ sort_chain_graph_abs(struct rb_root *rb_root, struct callchain_root *chain_root,
static void __sort_chain_graph_rel(struct callchain_node *node,
double min_percent)
{
+ struct rb_node *n;
struct callchain_node *child;
u64 min_hit;
node->rb_root = RB_ROOT;
min_hit = ceil(node->children_hit * min_percent);
- chain_for_each_child(child, node) {
+ n = rb_first(&node->rb_root_in);
+ while (n) {
+ child = rb_entry(n, struct callchain_node, rb_node_in);
+ n = rb_next(n);
+
__sort_chain_graph_rel(child, min_percent);
if (callchain_cumul_hits(child) >= min_hit)
rb_insert_callchain(&node->rb_root, child,
@@ -173,19 +183,26 @@ create_child(struct callchain_node *parent, bool inherit_children)
return NULL;
}
new->parent = parent;
- INIT_LIST_HEAD(&new->children);
INIT_LIST_HEAD(&new->val);
if (inherit_children) {
- struct callchain_node *next;
+ struct rb_node *n;
+ struct callchain_node *child;
+
+ new->rb_root_in = parent->rb_root_in;
+ parent->rb_root_in = RB_ROOT;
- list_splice(&parent->children, &new->children);
- INIT_LIST_HEAD(&parent->children);
+ n = rb_first(&new->rb_root_in);
+ while (n) {
+ child = rb_entry(n, struct callchain_node, rb_node_in);
+ child->parent = new;
+ n = rb_next(n);
+ }
- chain_for_each_child(next, new)
- next->parent = new;
+ /* make it the first child */
+ rb_link_node(&new->rb_node_in, NULL, &parent->rb_root_in.rb_node);
+ rb_insert_color(&new->rb_node_in, &parent->rb_root_in);
}
- list_add_tail(&new->siblings, &parent->children);
return new;
}
@@ -223,7 +240,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
}
}
-static void
+static struct callchain_node *
add_child(struct callchain_node *parent,
struct callchain_cursor *cursor,
u64 period)
@@ -235,6 +252,19 @@ add_child(struct callchain_node *parent,
new->children_hit = 0;
new->hit = period;
+ return new;
+}
+
+static s64 match_chain(struct callchain_cursor_node *node,
+ struct callchain_list *cnode)
+{
+ struct symbol *sym = node->sym;
+
+ if (cnode->ms.sym && sym &&
+ callchain_param.key == CCKEY_FUNCTION)
+ return cnode->ms.sym->start - sym->start;
+ else
+ return cnode->ip - node->ip;
}
/*
@@ -272,9 +302,33 @@ split_add_child(struct callchain_node *parent,
/* create a new child for the new branch if any */
if (idx_total < cursor->nr) {
+ struct callchain_node *first;
+ struct callchain_list *cnode;
+ struct callchain_cursor_node *node;
+ struct rb_node *p, **pp;
+
parent->hit = 0;
- add_child(parent, cursor, period);
parent->children_hit += period;
+
+ node = callchain_cursor_current(cursor);
+ new = add_child(parent, cursor, period);
+
+ /*
+ * This is second child since we moved parent's children
+ * to new (first) child above.
+ */
+ p = parent->rb_root_in.rb_node;
+ first = rb_entry(p, struct callchain_node, rb_node_in);
+ cnode = list_first_entry(&first->val, struct callchain_list,
+ list);
+
+ if (match_chain(node, cnode) < 0)
+ pp = &p->rb_left;
+ else
+ pp = &p->rb_right;
+
+ rb_link_node(&new->rb_node_in, p, pp);
+ rb_insert_color(&new->rb_node_in, &parent->rb_root_in);
} else {
parent->hit = period;
}
@@ -291,16 +345,40 @@ append_chain_children(struct callchain_node *root,
u64 period)
{
struct callchain_node *rnode;
+ struct callchain_cursor_node *node;
+ struct rb_node **p = &root->rb_root_in.rb_node;
+ struct rb_node *parent = NULL;
+
+ node = callchain_cursor_current(cursor);
+ if (!node)
+ return;
/* lookup in childrens */
- chain_for_each_child(rnode, root) {
- unsigned int ret = append_chain(rnode, cursor, period);
+ while (*p) {
+ s64 ret;
+ struct callchain_list *cnode;
- if (!ret)
+ parent = *p;
+ rnode = rb_entry(parent, struct callchain_node, rb_node_in);
+ cnode = list_first_entry(&rnode->val, struct callchain_list,
+ list);
+
+ /* just check first entry */
+ ret = match_chain(node, cnode);
+ if (ret == 0) {
+ append_chain(rnode, cursor, period);
goto inc_children_hit;
+ }
+
+ if (ret < 0)
+ p = &parent->rb_left;
+ else
+ p = &parent->rb_right;
}
/* nothing in children, add to the current node */
- add_child(root, cursor, period);
+ rnode = add_child(root, cursor, period);
+ rb_link_node(&rnode->rb_node_in, parent, p);
+ rb_insert_color(&rnode->rb_node_in, &root->rb_root_in);
inc_children_hit:
root->children_hit += period;
@@ -325,28 +403,20 @@ append_chain(struct callchain_node *root,
*/
list_for_each_entry(cnode, &root->val, list) {
struct callchain_cursor_node *node;
- struct symbol *sym;
node = callchain_cursor_current(cursor);
if (!node)
break;
- sym = node->sym;
-
- if (cnode->ms.sym && sym &&
- callchain_param.key == CCKEY_FUNCTION) {
- if (cnode->ms.sym->start != sym->start)
- break;
- } else if (cnode->ip != node->ip)
+ if (match_chain(node, cnode) != 0)
break;
- if (!found)
- found = true;
+ found = true;
callchain_cursor_advance(cursor);
}
- /* matches not, relay on the parent */
+ /* matches not, relay no the parent */
if (!found) {
cursor->curr = curr_snap;
cursor->pos = start;
@@ -395,8 +465,9 @@ merge_chain_branch(struct callchain_cursor *cursor,
struct callchain_node *dst, struct callchain_node *src)
{
struct callchain_cursor_node **old_last = cursor->last;
- struct callchain_node *child, *next_child;
+ struct callchain_node *child;
struct callchain_list *list, *next_list;
+ struct rb_node *n;
int old_pos = cursor->nr;
int err = 0;
@@ -412,12 +483,16 @@ merge_chain_branch(struct callchain_cursor *cursor,
append_chain_children(dst, cursor, src->hit);
}
- chain_for_each_child_safe(child, next_child, src) {
+ n = rb_first(&src->rb_root_in);
+ while (n) {
+ child = container_of(n, struct callchain_node, rb_node_in);
+ n = rb_next(n);
+ rb_erase(&child->rb_node_in, &src->rb_root_in);
+
err = merge_chain_branch(cursor, dst, child);
if (err)
break;
- list_del(&child->siblings);
free(child);
}
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 2b585bc308cf..7bb36022377f 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -21,11 +21,11 @@ enum chain_order {
struct callchain_node {
struct callchain_node *parent;
- struct list_head siblings;
- struct list_head children;
struct list_head val;
- struct rb_node rb_node; /* to sort nodes in an rbtree */
- struct rb_root rb_root; /* sorted tree of children */
+ struct rb_node rb_node_in; /* to insert nodes in an rbtree */
+ struct rb_node rb_node; /* to sort nodes in an output tree */
+ struct rb_root rb_root_in; /* input tree of children */
+ struct rb_root rb_root; /* sorted output tree of children */
unsigned int val_nr;
u64 hit;
u64 children_hit;
@@ -86,13 +86,12 @@ extern __thread struct callchain_cursor callchain_cursor;
static inline void callchain_init(struct callchain_root *root)
{
- INIT_LIST_HEAD(&root->node.siblings);
- INIT_LIST_HEAD(&root->node.children);
INIT_LIST_HEAD(&root->node.val);
root->node.parent = NULL;
root->node.hit = 0;
root->node.children_hit = 0;
+ root->node.rb_root_in = RB_ROOT;
root->max_depth = 0;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 1/8] perf callchain: Convert children list to rbtree
2013-09-26 8:58 ` [PATCH 1/8] perf callchain: Convert children list to rbtree Namhyung Kim
@ 2013-10-02 10:18 ` Frederic Weisbecker
2013-10-08 2:03 ` Namhyung Kim
0 siblings, 1 reply; 24+ messages in thread
From: Frederic Weisbecker @ 2013-10-02 10:18 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Linus Torvalds, Jiri Olsa
On Thu, Sep 26, 2013 at 05:58:03PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
>
> Current collapse stage has a scalability problem which can be
> reproduced easily with parallel kernel build. This is because it
> needs to traverse every children of callchain linearly during the
> collapse/merge stage. Convert it to rbtree reduced the overhead
> significantly.
>
> On my 400MB perf.data file which recorded with make -j32 kernel build:
>
> $ time perf --no-pager report --stdio > /dev/null
>
> before:
> real 6m22.073s
> user 6m18.683s
> sys 0m0.706s
>
> after:
> real 0m20.780s
> user 0m19.962s
> sys 0m0.689s
>
> During the perf report the overhead on append_chain_children went down
> from 96.69% to 18.16%:
>
> - 18.16% perf perf [.] append_chain_children
> - append_chain_children
> - 77.48% append_chain_children
> + 69.79% merge_chain_branch
> - 22.96% append_chain_children
> + 67.44% merge_chain_branch
> + 30.15% append_chain_children
> + 2.41% callchain_append
> + 7.25% callchain_append
> + 12.26% callchain_append
> + 10.22% merge_chain_branch
> + 11.58% perf perf [.] dso__find_symbol
> + 8.02% perf perf [.] sort__comm_cmp
> + 5.48% perf libc-2.17.so [.] malloc_consolidate
>
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Link: http://lkml.kernel.org/n/tip-d9tcfow6stbrp4btvgs51y67@git.kernel.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Have you tested this patchset when collapsing is not used?
There are fair chances that this patchset does not only improve collapsing
but also callchain insertion in general. So it's probably a win in any case. But
still it would be nice to make sure that it's the case because we are getting
rid of collapsing anyway.
The test that could tell us about that is to run "perf report -s sym" and compare the
time it takes to complete before and after this patch, because "-s sym" shouldn't
involve collapses.
Sorting by anything that is not comm should do the trick in fact.
Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] perf callchain: Convert children list to rbtree
2013-10-02 10:18 ` Frederic Weisbecker
@ 2013-10-08 2:03 ` Namhyung Kim
2013-10-08 19:22 ` Frederic Weisbecker
0 siblings, 1 reply; 24+ messages in thread
From: Namhyung Kim @ 2013-10-08 2:03 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Linus Torvalds, Jiri Olsa
On Wed, 2 Oct 2013 12:18:28 +0200, Frederic Weisbecker wrote:
> On Thu, Sep 26, 2013 at 05:58:03PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim <namhyung.kim@lge.com>
>>
>> Current collapse stage has a scalability problem which can be
>> reproduced easily with parallel kernel build. This is because it
>> needs to traverse every children of callchain linearly during the
>> collapse/merge stage. Convert it to rbtree reduced the overhead
>> significantly.
>>
>> On my 400MB perf.data file which recorded with make -j32 kernel build:
>>
>> $ time perf --no-pager report --stdio > /dev/null
>>
>> before:
>> real 6m22.073s
>> user 6m18.683s
>> sys 0m0.706s
>>
>> after:
>> real 0m20.780s
>> user 0m19.962s
>> sys 0m0.689s
>>
>> During the perf report the overhead on append_chain_children went down
>> from 96.69% to 18.16%:
>>
>> - 18.16% perf perf [.] append_chain_children
>> - append_chain_children
>> - 77.48% append_chain_children
>> + 69.79% merge_chain_branch
>> - 22.96% append_chain_children
>> + 67.44% merge_chain_branch
>> + 30.15% append_chain_children
>> + 2.41% callchain_append
>> + 7.25% callchain_append
>> + 12.26% callchain_append
>> + 10.22% merge_chain_branch
>> + 11.58% perf perf [.] dso__find_symbol
>> + 8.02% perf perf [.] sort__comm_cmp
>> + 5.48% perf libc-2.17.so [.] malloc_consolidate
>>
>> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Link: http://lkml.kernel.org/n/tip-d9tcfow6stbrp4btvgs51y67@git.kernel.org
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> Have you tested this patchset when collapsing is not used?
> There are fair chances that this patchset does not only improve collapsing
> but also callchain insertion in general. So it's probably a win in any case. But
> still it would be nice to make sure that it's the case because we are getting
> rid of collapsing anyway.
>
> The test that could tell us about that is to run "perf report -s sym" and compare the
> time it takes to complete before and after this patch, because "-s sym" shouldn't
> involve collapses.
>
> Sorting by anything that is not comm should do the trick in fact.
Yes, I have similar result when collapsing is not used. Actually when I
ran "perf report -s sym", the performance improves higher since it'd
insert more callchains in a hist entry.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] perf callchain: Convert children list to rbtree
2013-10-08 2:03 ` Namhyung Kim
@ 2013-10-08 19:22 ` Frederic Weisbecker
2013-10-10 1:06 ` Namhyung Kim
0 siblings, 1 reply; 24+ messages in thread
From: Frederic Weisbecker @ 2013-10-08 19:22 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Linus Torvalds, Jiri Olsa
On Tue, Oct 08, 2013 at 11:03:16AM +0900, Namhyung Kim wrote:
> On Wed, 2 Oct 2013 12:18:28 +0200, Frederic Weisbecker wrote:
> > On Thu, Sep 26, 2013 at 05:58:03PM +0900, Namhyung Kim wrote:
> >> From: Namhyung Kim <namhyung.kim@lge.com>
> >>
> >> Current collapse stage has a scalability problem which can be
> >> reproduced easily with parallel kernel build. This is because it
> >> needs to traverse every children of callchain linearly during the
> >> collapse/merge stage. Convert it to rbtree reduced the overhead
> >> significantly.
> >>
> >> On my 400MB perf.data file which recorded with make -j32 kernel build:
> >>
> >> $ time perf --no-pager report --stdio > /dev/null
> >>
> >> before:
> >> real 6m22.073s
> >> user 6m18.683s
> >> sys 0m0.706s
> >>
> >> after:
> >> real 0m20.780s
> >> user 0m19.962s
> >> sys 0m0.689s
> >>
> >> During the perf report the overhead on append_chain_children went down
> >> from 96.69% to 18.16%:
> >>
> >> - 18.16% perf perf [.] append_chain_children
> >> - append_chain_children
> >> - 77.48% append_chain_children
> >> + 69.79% merge_chain_branch
> >> - 22.96% append_chain_children
> >> + 67.44% merge_chain_branch
> >> + 30.15% append_chain_children
> >> + 2.41% callchain_append
> >> + 7.25% callchain_append
> >> + 12.26% callchain_append
> >> + 10.22% merge_chain_branch
> >> + 11.58% perf perf [.] dso__find_symbol
> >> + 8.02% perf perf [.] sort__comm_cmp
> >> + 5.48% perf libc-2.17.so [.] malloc_consolidate
> >>
> >> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> >> Cc: Jiri Olsa <jolsa@redhat.com>
> >> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >> Link: http://lkml.kernel.org/n/tip-d9tcfow6stbrp4btvgs51y67@git.kernel.org
> >> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >
> > Have you tested this patchset when collapsing is not used?
> > There are fair chances that this patchset does not only improve collapsing
> > but also callchain insertion in general. So it's probably a win in any case. But
> > still it would be nice to make sure that it's the case because we are getting
> > rid of collapsing anyway.
> >
> > The test that could tell us about that is to run "perf report -s sym" and compare the
> > time it takes to complete before and after this patch, because "-s sym" shouldn't
> > involve collapses.
> >
> > Sorting by anything that is not comm should do the trick in fact.
>
> Yes, I have similar result when collapsing is not used. Actually when I
> ran "perf report -s sym", the performance improves higher since it'd
> insert more callchains in a hist entry.
Great! I'll have a closer look and review on the callchain patches then. Please
resend these along the comm batch.
Thanks again!
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/8] perf callchain: Convert children list to rbtree
2013-10-08 19:22 ` Frederic Weisbecker
@ 2013-10-10 1:06 ` Namhyung Kim
0 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-10-10 1:06 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Linus Torvalds, Jiri Olsa
Hi Frederic,
On Tue, 8 Oct 2013 21:22:45 +0200, Frederic Weisbecker wrote:
> On Tue, Oct 08, 2013 at 11:03:16AM +0900, Namhyung Kim wrote:
>> On Wed, 2 Oct 2013 12:18:28 +0200, Frederic Weisbecker wrote:
>> > Have you tested this patchset when collapsing is not used?
>> > There are fair chances that this patchset does not only improve collapsing
>> > but also callchain insertion in general. So it's probably a win in any case. But
>> > still it would be nice to make sure that it's the case because we are getting
>> > rid of collapsing anyway.
>> >
>> > The test that could tell us about that is to run "perf report -s sym" and compare the
>> > time it takes to complete before and after this patch, because "-s sym" shouldn't
>> > involve collapses.
>> >
>> > Sorting by anything that is not comm should do the trick in fact.
>>
>> Yes, I have similar result when collapsing is not used. Actually when I
>> ran "perf report -s sym", the performance improves higher since it'd
>> insert more callchains in a hist entry.
>
> Great! I'll have a closer look and review on the callchain patches then. Please
> resend these along the comm batch.
I'll do it later today or tomorrow.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/8] perf ui/progress: Add new helper functions for progress bar
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
2013-09-26 8:58 ` [PATCH 1/8] perf callchain: Convert children list to rbtree Namhyung Kim
@ 2013-09-26 8:58 ` Namhyung Kim
2013-09-26 8:58 ` [PATCH 3/8] perf tools: Show progress on histogram collapsing Namhyung Kim
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-09-26 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Linus Torvalds, Frederic Weisbecker, Jiri Olsa
From: Namhyung Kim <namhyung.kim@lge.com>
Introduce ui_progress_setup() and ui_progress__advance() to separate
out from the session logic. It'll be used by other places in the
upcoming patch.
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/progress.c | 18 ++++++++++++++++++
tools/perf/ui/progress.h | 10 ++++++++++
tools/perf/util/session.c | 24 ++++++++++++------------
3 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/tools/perf/ui/progress.c b/tools/perf/ui/progress.c
index 3ec695607a4d..b5c22c6b1d93 100644
--- a/tools/perf/ui/progress.c
+++ b/tools/perf/ui/progress.c
@@ -19,6 +19,24 @@ void ui_progress__update(u64 curr, u64 total, const char *title)
return progress_fns->update(curr, total, title);
}
+void ui_progress__setup(struct perf_progress *p, u64 total)
+{
+ p->curr = 0;
+ p->next = p->unit = total / 16;
+ p->total = total;
+
+}
+
+void ui_progress__advance(struct perf_progress *p, u64 adv, const char *title)
+{
+ p->curr += adv;
+
+ if (p->curr >= p->next) {
+ p->next += p->unit;
+ ui_progress__update(p->curr, p->total, title);
+ }
+}
+
void ui_progress__finish(void)
{
if (progress_fns->finish)
diff --git a/tools/perf/ui/progress.h b/tools/perf/ui/progress.h
index 257cc224f9cf..3bd23e825953 100644
--- a/tools/perf/ui/progress.h
+++ b/tools/perf/ui/progress.h
@@ -8,10 +8,20 @@ struct ui_progress {
void (*finish)(void);
};
+struct perf_progress {
+ u64 curr;
+ u64 next;
+ u64 unit;
+ u64 total;
+};
+
extern struct ui_progress *progress_fns;
void ui_progress__init(void);
+void ui_progress__setup(struct perf_progress *p, u64 total);
+void ui_progress__advance(struct perf_progress *p, u64 adv, const char *title);
+
void ui_progress__update(u64 curr, u64 total, const char *title);
void ui_progress__finish(void);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 6c1d4447c5b4..9f62be8bc167 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -538,13 +538,16 @@ static int flush_sample_queue(struct perf_session *s,
struct perf_sample sample;
u64 limit = os->next_flush;
u64 last_ts = os->last_sample ? os->last_sample->timestamp : 0ULL;
- unsigned idx = 0, progress_next = os->nr_samples / 16;
bool show_progress = limit == ULLONG_MAX;
+ struct perf_progress prog;
int ret;
if (!tool->ordered_samples || !limit)
return 0;
+ if (show_progress)
+ ui_progress__setup(&prog, os->nr_samples);
+
list_for_each_entry_safe(iter, tmp, head, list) {
if (iter->timestamp > limit)
break;
@@ -562,10 +565,10 @@ static int flush_sample_queue(struct perf_session *s,
os->last_flush = iter->timestamp;
list_del(&iter->list);
list_add(&iter->list, &os->sample_cache);
- if (show_progress && (++idx >= progress_next)) {
- progress_next += os->nr_samples / 16;
- ui_progress__update(idx, os->nr_samples,
- "Processing time ordered events...");
+
+ if (show_progress) {
+ ui_progress__advance(&prog, 1,
+ "Processing time ordered events...");
}
}
@@ -1310,12 +1313,13 @@ int __perf_session__process_events(struct perf_session *session,
u64 data_offset, u64 data_size,
u64 file_size, struct perf_tool *tool)
{
- u64 head, page_offset, file_offset, file_pos, progress_next;
+ u64 head, page_offset, file_offset, file_pos;
int err, mmap_prot, mmap_flags, map_idx = 0;
size_t mmap_size;
char *buf, *mmaps[NUM_MMAPS];
union perf_event *event;
uint32_t size;
+ struct perf_progress prog;
perf_tool__fill_defaults(tool);
@@ -1326,7 +1330,7 @@ int __perf_session__process_events(struct perf_session *session,
if (data_offset + data_size < file_size)
file_size = data_offset + data_size;
- progress_next = file_size / 16;
+ ui_progress__setup(&prog, file_size);
mmap_size = MMAP_SIZE;
if (mmap_size > file_size)
@@ -1381,11 +1385,7 @@ more:
head += size;
file_pos += size;
- if (file_pos >= progress_next) {
- progress_next += file_size / 16;
- ui_progress__update(file_pos, file_size,
- "Processing events...");
- }
+ ui_progress__advance(&prog, size, "Processing events...");
if (file_pos < file_size)
goto more;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 3/8] perf tools: Show progress on histogram collapsing
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
2013-09-26 8:58 ` [PATCH 1/8] perf callchain: Convert children list to rbtree Namhyung Kim
2013-09-26 8:58 ` [PATCH 2/8] perf ui/progress: Add new helper functions for progress bar Namhyung Kim
@ 2013-09-26 8:58 ` Namhyung Kim
2013-09-26 8:58 ` [PATCH 4/8] perf tools: Use an accessor to read thread comm Namhyung Kim
` (6 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-09-26 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Linus Torvalds, Frederic Weisbecker, Jiri Olsa
From: Namhyung Kim <namhyung.kim@lge.com>
It can take quite amount of time so add progress bar UI to inform user.
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-annotate.c | 2 +-
tools/perf/builtin-diff.c | 2 +-
tools/perf/builtin-report.c | 10 +++++++++-
tools/perf/builtin-top.c | 4 ++--
tools/perf/tests/hists_link.c | 2 +-
tools/perf/util/hist.c | 5 ++++-
tools/perf/util/hist.h | 3 ++-
7 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 577c3a1cffea..958d5e280b1f 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -242,7 +242,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
if (nr_samples > 0) {
total_nr_samples += nr_samples;
- hists__collapse_resort(hists);
+ hists__collapse_resort(hists, NULL);
hists__output_resort(hists);
if (symbol_conf.event_group &&
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f28799e94f2a..f4ab7d210105 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -367,7 +367,7 @@ static void perf_evlist__collapse_resort(struct perf_evlist *evlist)
list_for_each_entry(evsel, &evlist->entries, node) {
struct hists *hists = &evsel->hists;
- hists__collapse_resort(hists);
+ hists__collapse_resort(hists, NULL);
}
}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 295025e6b26e..3390d9a4e3bf 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -489,6 +489,7 @@ static int __cmd_report(struct perf_report *rep)
struct map *kernel_map;
struct kmap *kernel_kmap;
const char *help = "For a higher level overview, try: perf report --sort comm,dso";
+ struct perf_progress prog;
signal(SIGINT, sig_handler);
@@ -550,13 +551,19 @@ static int __cmd_report(struct perf_report *rep)
}
nr_samples = 0;
+ list_for_each_entry(pos, &session->evlist->entries, node)
+ nr_samples += pos->hists.nr_entries;
+
+ ui_progress__setup(&prog, nr_samples);
+
+ nr_samples = 0;
list_for_each_entry(pos, &session->evlist->entries, node) {
struct hists *hists = &pos->hists;
if (pos->idx == 0)
hists->symbol_filter_str = rep->symbol_filter_str;
- hists__collapse_resort(hists);
+ hists__collapse_resort(hists, &prog);
nr_samples += hists->stats.nr_events[PERF_RECORD_SAMPLE];
/* Non-group events are considered as leader */
@@ -568,6 +575,7 @@ static int __cmd_report(struct perf_report *rep)
hists__link(leader_hists, hists);
}
}
+ ui_progress__finish();
if (nr_samples == 0) {
ui__error("The %s file has no samples!\n", session->filename);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index cf4bba07175a..0cb077f823de 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -287,7 +287,7 @@ static void perf_top__print_sym_table(struct perf_top *top)
return;
}
- hists__collapse_resort(&top->sym_evsel->hists);
+ hists__collapse_resort(&top->sym_evsel->hists, NULL);
hists__output_resort(&top->sym_evsel->hists);
hists__decay_entries(&top->sym_evsel->hists,
top->hide_user_symbols,
@@ -553,7 +553,7 @@ static void perf_top__sort_new_samples(void *arg)
if (t->evlist->selected != NULL)
t->sym_evsel = t->evlist->selected;
- hists__collapse_resort(&t->sym_evsel->hists);
+ hists__collapse_resort(&t->sym_evsel->hists, NULL);
hists__output_resort(&t->sym_evsel->hists);
hists__decay_entries(&t->sym_evsel->hists,
t->hide_user_symbols,
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 4228ffc0d968..3b5faaa5c8cf 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -465,7 +465,7 @@ int test__hists_link(void)
goto out;
list_for_each_entry(evsel, &evlist->entries, node) {
- hists__collapse_resort(&evsel->hists);
+ hists__collapse_resort(&evsel->hists, NULL);
if (verbose > 2)
print_hists(&evsel->hists);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fa695ce1662a..a2d58e7abdcf 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -395,6 +395,7 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
if (!he)
return NULL;
+ hists->nr_entries++;
rb_link_node(&he->rb_node_in, parent, p);
rb_insert_color(&he->rb_node_in, hists->entries_in);
out:
@@ -599,7 +600,7 @@ static void hists__apply_filters(struct hists *hists, struct hist_entry *he)
hists__filter_entry_by_symbol(hists, he);
}
-void hists__collapse_resort(struct hists *hists)
+void hists__collapse_resort(struct hists *hists, struct perf_progress *prog)
{
struct rb_root *root;
struct rb_node *next;
@@ -624,6 +625,8 @@ void hists__collapse_resort(struct hists *hists)
*/
hists__apply_filters(hists, n);
}
+ if (prog)
+ ui_progress__advance(prog, 1, "Merging related events...");
}
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 7f29792efc58..f3d6cf6bd7d4 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -5,6 +5,7 @@
#include <pthread.h>
#include "callchain.h"
#include "header.h"
+#include "ui/progress.h"
extern struct callchain_param callchain_param;
@@ -104,7 +105,7 @@ struct hist_entry *__hists__add_mem_entry(struct hists *self,
u64 weight);
void hists__output_resort(struct hists *self);
-void hists__collapse_resort(struct hists *self);
+void hists__collapse_resort(struct hists *self, struct perf_progress *prog);
void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel);
void hists__output_recalc_col_len(struct hists *hists, int max_rows);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 4/8] perf tools: Use an accessor to read thread comm
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
` (2 preceding siblings ...)
2013-09-26 8:58 ` [PATCH 3/8] perf tools: Show progress on histogram collapsing Namhyung Kim
@ 2013-09-26 8:58 ` Namhyung Kim
2013-09-26 8:58 ` [PATCH 5/8] perf tools: Add time argument on comm setting Namhyung Kim
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-09-26 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Linus Torvalds, Frederic Weisbecker, Jiri Olsa, David Ahern,
Ingo Molnar, Arnaldo Carvalho de Melo, Stephane Eranian
From: Frederic Weisbecker <fweisbec@gmail.com>
As the thread comm is going to be implemented by way of a more
complicated data structure than just a pointer to a string from the
thread struct, convert the readers of comm to use an accessor instead
of accessing it directly. The accessor will be later overriden to
support an enhanced comm implementation.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-wr683zwy94hmj4ibogmnv9ce@git.kernel.org
[ Fixed up some minor const pointer issues ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-kmem.c | 2 +-
tools/perf/builtin-lock.c | 2 +-
tools/perf/builtin-sched.c | 16 ++++++++--------
tools/perf/builtin-script.c | 6 +++---
tools/perf/builtin-trace.c | 5 +++--
tools/perf/tests/hists_link.c | 2 +-
tools/perf/ui/browsers/hists.c | 10 +++++-----
tools/perf/util/event.c | 4 ++--
tools/perf/util/scripting-engines/trace-event-perl.c | 2 +-
tools/perf/util/scripting-engines/trace-event-python.c | 4 ++--
tools/perf/util/sort.c | 10 +++++-----
tools/perf/util/thread.c | 7 ++++++-
tools/perf/util/thread.h | 1 +
13 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 9b5f077fee5b..1a85296f6af1 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -314,7 +314,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
return -1;
}
- dump_printf(" ... thread: %s:%d\n", thread->comm, thread->tid);
+ dump_printf(" ... thread: %s:%d\n", thread__comm_curr(thread), thread->tid);
if (evsel->handler.func != NULL) {
tracepoint_handler f = evsel->handler.func;
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 6a9076f165f4..69670c6bc165 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -766,7 +766,7 @@ static void dump_threads(void)
while (node) {
st = container_of(node, struct thread_stat, rb);
t = perf_session__findnew(session, st->tid);
- pr_info("%10d: %s\n", st->tid, t->comm);
+ pr_info("%10d: %s\n", st->tid, thread__comm_curr(t));
node = rb_next(node);
};
}
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index d8c51b2f263f..2699719dfde2 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -737,12 +737,12 @@ static int replay_fork_event(struct perf_sched *sched,
if (verbose) {
printf("fork event\n");
- printf("... parent: %s/%d\n", parent->comm, parent->tid);
- printf("... child: %s/%d\n", child->comm, child->tid);
+ printf("... parent: %s/%d\n", thread__comm_curr(parent), parent->tid);
+ printf("... child: %s/%d\n", thread__comm_curr(child), child->tid);
}
- register_pid(sched, parent->tid, parent->comm);
- register_pid(sched, child->tid, child->comm);
+ register_pid(sched, parent->tid, thread__comm_curr(parent));
+ register_pid(sched, child->tid, thread__comm_curr(child));
return 0;
}
@@ -1077,7 +1077,7 @@ static int latency_migrate_task_event(struct perf_sched *sched,
if (!atoms) {
if (thread_atoms_insert(sched, migrant))
return -1;
- register_pid(sched, migrant->tid, migrant->comm);
+ register_pid(sched, migrant->tid, thread__comm_curr(migrant));
atoms = thread_atoms_search(&sched->atom_root, migrant, &sched->cmp_pid);
if (!atoms) {
pr_err("migration-event: Internal tree error");
@@ -1111,13 +1111,13 @@ static void output_lat_thread(struct perf_sched *sched, struct work_atoms *work_
/*
* Ignore idle threads:
*/
- if (!strcmp(work_list->thread->comm, "swapper"))
+ if (!strcmp(thread__comm_curr(work_list->thread), "swapper"))
return;
sched->all_runtime += work_list->total_runtime;
sched->all_count += work_list->nb_atoms;
- ret = printf(" %s:%d ", work_list->thread->comm, work_list->thread->tid);
+ ret = printf(" %s:%d ", thread__comm_curr(work_list->thread), work_list->thread->tid);
for (i = 0; i < 24 - ret; i++)
printf(" ");
@@ -1334,7 +1334,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
printf(" %12.6f secs ", (double)timestamp/1e9);
if (new_shortname) {
printf("%s => %s:%d\n",
- sched_in->shortname, sched_in->comm, sched_in->tid);
+ sched_in->shortname, thread__comm_curr(sched_in), sched_in->tid);
} else {
printf("\n");
}
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 7f31a3ded1b6..50258aa8cea1 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -290,11 +290,11 @@ static void print_sample_start(struct perf_sample *sample,
if (PRINT_FIELD(COMM)) {
if (latency_format)
- printf("%8.8s ", thread->comm);
+ printf("%8.8s ", thread__comm_curr(thread));
else if (PRINT_FIELD(IP) && symbol_conf.use_callchain)
- printf("%s ", thread->comm);
+ printf("%s ", thread__comm_curr(thread));
else
- printf("%16s ", thread->comm);
+ printf("%16s ", thread__comm_curr(thread));
}
if (PRINT_FIELD(PID) && PRINT_FIELD(TID))
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 1bb8f154852a..a96fd47b369b 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1025,7 +1025,8 @@ static size_t trace__fprintf_entry_head(struct trace *trace, struct thread *thre
if (trace->multiple_threads) {
if (trace->show_comm)
- printed += fprintf(fp, "%.14s/", thread->comm);
+ printed += fprintf(fp, "%.14s/",
+ thread__comm_curr(thread));
printed += fprintf(fp, "%d ", thread->tid);
}
@@ -1730,7 +1731,7 @@ static size_t trace__fprintf_thread_summary(struct trace *trace, FILE *fp)
else if (ratio > 5.0)
color = PERF_COLOR_YELLOW;
- printed += color_fprintf(fp, color, "%20s", thread->comm);
+ printed += color_fprintf(fp, color, "%20s", thread__comm_curr(thread));
printed += fprintf(fp, " - %-5d :%11lu [", thread->tid, ttrace->nr_events);
printed += color_fprintf(fp, color, "%5.1f%%", ratio);
printed += fprintf(fp, " ] %10.3f ms\n", ttrace->runtime_ms);
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 3b5faaa5c8cf..735cff2c52ce 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -419,7 +419,7 @@ static void print_hists(struct hists *hists)
he = rb_entry(node, struct hist_entry, rb_node_in);
pr_info("%2d: entry: %-8s [%-8s] %20s: period = %"PRIu64"\n",
- i, he->thread->comm, he->ms.map->dso->short_name,
+ i, thread__comm_curr(he->thread), he->ms.map->dso->short_name,
he->ms.sym->name, he->stat.period);
i++;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 7ef36c360471..abb68e24d858 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1255,7 +1255,7 @@ static int hists__browser_title(struct hists *hists, char *bf, size_t size,
if (thread)
printed += scnprintf(bf + printed, size - printed,
", Thread: %s(%d)",
- (thread->comm_set ? thread->comm : ""),
+ (thread->comm_set ? thread__comm_curr(thread) : ""),
thread->tid);
if (dso)
printed += scnprintf(bf + printed, size - printed,
@@ -1578,7 +1578,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (thread != NULL &&
asprintf(&options[nr_options], "Zoom %s %s(%d) thread",
(browser->hists->thread_filter ? "out of" : "into"),
- (thread->comm_set ? thread->comm : ""),
+ (thread->comm_set ? thread__comm_curr(thread) : ""),
thread->tid) > 0)
zoom_thread = nr_options++;
@@ -1598,7 +1598,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
struct symbol *sym;
if (asprintf(&options[nr_options], "Run scripts for samples of thread [%s]",
- browser->he_selection->thread->comm) > 0)
+ thread__comm_curr(browser->he_selection->thread)) > 0)
scripts_comm = nr_options++;
sym = browser->he_selection->ms.sym;
@@ -1701,7 +1701,7 @@ zoom_out_thread:
sort_thread.elide = false;
} else {
ui_helpline__fpush("To zoom out press <- or -> + \"Zoom out of %s(%d) thread\"",
- thread->comm_set ? thread->comm : "",
+ thread->comm_set ? thread__comm_curr(thread) : "",
thread->tid);
browser->hists->thread_filter = thread;
sort_thread.elide = true;
@@ -1717,7 +1717,7 @@ do_scripts:
memset(script_opt, 0, 64);
if (choice == scripts_comm)
- sprintf(script_opt, " -c %s ", browser->he_selection->thread->comm);
+ sprintf(script_opt, " -c %s ", thread__comm_curr(browser->he_selection->thread));
if (choice == scripts_symbol)
sprintf(script_opt, " -S %s ", browser->he_selection->ms.sym->name);
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 9b393e7dca6f..d639bfe29309 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -725,10 +725,10 @@ int perf_event__preprocess_sample(const union perf_event *event,
return -1;
if (symbol_conf.comm_list &&
- !strlist__has_entry(symbol_conf.comm_list, thread->comm))
+ !strlist__has_entry(symbol_conf.comm_list, thread__comm_curr(thread)))
goto out_filtered;
- dump_printf(" ... thread: %s:%d\n", thread->comm, thread->tid);
+ dump_printf(" ... thread: %s:%d\n", thread__comm_curr(thread), thread->tid);
/*
* Have we already created the kernel maps for this machine?
*
diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
index a85e4ae5f3ac..c4065ed3ae5a 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -273,7 +273,7 @@ static void perl_process_tracepoint(union perf_event *perf_event __maybe_unused,
int cpu = sample->cpu;
void *data = sample->raw_data;
unsigned long long nsecs = sample->time;
- char *comm = thread->comm;
+ const char *comm = thread__comm_curr(thread);
dSP;
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index cc75a3cef388..b532675be3c1 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -239,7 +239,7 @@ static void python_process_tracepoint(union perf_event *perf_event
int cpu = sample->cpu;
void *data = sample->raw_data;
unsigned long long nsecs = sample->time;
- char *comm = thread->comm;
+ const char *comm = thread__comm_curr(thread);
t = PyTuple_New(MAX_FIELDS);
if (!t)
@@ -378,7 +378,7 @@ static void python_process_general_event(union perf_event *perf_event
PyDict_SetItemString(dict, "raw_buf", PyString_FromStringAndSize(
(const char *)sample->raw_data, sample->raw_size));
PyDict_SetItemString(dict, "comm",
- PyString_FromString(thread->comm));
+ PyString_FromString(thread__comm_curr(thread)));
if (al->map) {
PyDict_SetItemString(dict, "dso",
PyString_FromString(al->map->dso->name));
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 082480579861..65fe958ce30b 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -42,7 +42,7 @@ static int repsep_snprintf(char *bf, size_t size, const char *fmt, ...)
return n;
}
-static int64_t cmp_null(void *l, void *r)
+static int64_t cmp_null(const void *l, const void *r)
{
if (!l && !r)
return 0;
@@ -64,7 +64,7 @@ static int hist_entry__thread_snprintf(struct hist_entry *self, char *bf,
size_t size, unsigned int width)
{
return repsep_snprintf(bf, size, "%*s:%5d", width - 6,
- self->thread->comm ?: "", self->thread->tid);
+ thread__comm_curr(self->thread) ?: "", self->thread->tid);
}
struct sort_entry sort_thread = {
@@ -85,8 +85,8 @@ sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
static int64_t
sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
{
- char *comm_l = left->thread->comm;
- char *comm_r = right->thread->comm;
+ const char *comm_l = thread__comm_curr(left->thread);
+ const char *comm_r = thread__comm_curr(right->thread);
if (!comm_l || !comm_r)
return cmp_null(comm_l, comm_r);
@@ -97,7 +97,7 @@ sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
static int hist_entry__comm_snprintf(struct hist_entry *self, char *bf,
size_t size, unsigned int width)
{
- return repsep_snprintf(bf, size, "%*s", width, self->thread->comm);
+ return repsep_snprintf(bf, size, "%*s", width, thread__comm_curr(self->thread));
}
struct sort_entry sort_comm = {
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index e3d4a550a703..bb50b9fda9c2 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -45,6 +45,11 @@ int thread__set_comm(struct thread *self, const char *comm)
return err;
}
+const char *thread__comm_curr(const struct thread *thread)
+{
+ return thread->comm;
+}
+
int thread__comm_len(struct thread *self)
{
if (!self->comm_len) {
@@ -58,7 +63,7 @@ int thread__comm_len(struct thread *self)
size_t thread__fprintf(struct thread *thread, FILE *fp)
{
- return fprintf(fp, "Thread %d %s\n", thread->tid, thread->comm) +
+ return fprintf(fp, "Thread %d %s\n", thread->tid, thread__comm_curr(thread)) +
map_groups__fprintf(&thread->mg, verbose, fp);
}
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 4ebbb40d46d4..84db214d90bd 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -35,6 +35,7 @@ static inline void thread__exited(struct thread *thread)
int thread__set_comm(struct thread *self, const char *comm);
int thread__comm_len(struct thread *self);
+const char *thread__comm_curr(const struct thread *thread);
void thread__insert_map(struct thread *self, struct map *map);
int thread__fork(struct thread *self, struct thread *parent);
size_t thread__fprintf(struct thread *thread, FILE *fp);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 5/8] perf tools: Add time argument on comm setting
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
` (3 preceding siblings ...)
2013-09-26 8:58 ` [PATCH 4/8] perf tools: Use an accessor to read thread comm Namhyung Kim
@ 2013-09-26 8:58 ` Namhyung Kim
2013-09-26 8:58 ` [PATCH 6/8] perf tools: Add new comm infrastructure Namhyung Kim
` (4 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-09-26 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Linus Torvalds, Frederic Weisbecker, Jiri Olsa, David Ahern,
Ingo Molnar, Stephane Eranian, Arnaldo Carvalho de Melo
From: Frederic Weisbecker <fweisbec@gmail.com>
This way we can later delimit a lifecycle for the comm and map a hist to
a precise comm:timeslice couple.
Comm and fork events that don't have PERF_SAMPLE_TIME samples can only
send 0 value as a timestamp and thus should overwrite any previous comm
on a given thread because there is no sensible way to keep track of all
the comms lifecycles in a thread without time informations.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-6tyow99vgmmtt9qwr2u2lqd7@git.kernel.org
[ Made it cope with PERF_RECORD_MMAP2 ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-top.c | 2 +-
tools/perf/builtin-trace.c | 12 ++++++------
tools/perf/tests/code-reading.c | 2 +-
tools/perf/tests/hists_link.c | 4 ++--
tools/perf/util/event.c | 28 ++++++++++++++--------------
tools/perf/util/machine.c | 39 ++++++++++++++++++++++-----------------
tools/perf/util/machine.h | 21 ++++++++++++++-------
tools/perf/util/session.c | 2 +-
tools/perf/util/thread.c | 6 ++++--
tools/perf/util/thread.h | 4 ++--
10 files changed, 67 insertions(+), 53 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0cb077f823de..bb5364c87c3f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -856,7 +856,7 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
&sample, machine);
} else if (event->header.type < PERF_RECORD_MAX) {
hists__inc_nr_events(&evsel->hists, event->header.type);
- machine__process_event(machine, event);
+ machine__process_event(machine, event, &sample);
} else
++session->stats.nr_unknown_events;
}
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index a96fd47b369b..f1d0dc48ba34 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1034,7 +1034,7 @@ static size_t trace__fprintf_entry_head(struct trace *trace, struct thread *thre
}
static int trace__process_event(struct trace *trace, struct machine *machine,
- union perf_event *event)
+ union perf_event *event, struct perf_sample *sample)
{
int ret = 0;
@@ -1042,9 +1042,9 @@ static int trace__process_event(struct trace *trace, struct machine *machine,
case PERF_RECORD_LOST:
color_fprintf(trace->output, PERF_COLOR_RED,
"LOST %" PRIu64 " events!\n", event->lost.lost);
- ret = machine__process_lost_event(machine, event);
+ ret = machine__process_lost_event(machine, event, sample);
default:
- ret = machine__process_event(machine, event);
+ ret = machine__process_event(machine, event, sample);
break;
}
@@ -1053,11 +1053,11 @@ static int trace__process_event(struct trace *trace, struct machine *machine,
static int trace__tool_process(struct perf_tool *tool,
union perf_event *event,
- struct perf_sample *sample __maybe_unused,
+ struct perf_sample *sample,
struct machine *machine)
{
struct trace *trace = container_of(tool, struct trace, tool);
- return trace__process_event(trace, machine, event);
+ return trace__process_event(trace, machine, event, sample);
}
static int trace__symbols_init(struct trace *trace, struct perf_evlist *evlist)
@@ -1581,7 +1581,7 @@ again:
trace->base_time = sample.time;
if (type != PERF_RECORD_SAMPLE) {
- trace__process_event(trace, &trace->host, event);
+ trace__process_event(trace, &trace->host, event, &sample);
continue;
}
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 6fb781d5586c..38d233a27de6 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -276,7 +276,7 @@ static int process_event(struct machine *machine, struct perf_evlist *evlist,
return process_sample_event(machine, evlist, event, state);
if (event->header.type < PERF_RECORD_MAX)
- return machine__process_event(machine, event);
+ return machine__process_event(machine, event, NULL);
return 0;
}
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 735cff2c52ce..56f811d55a6f 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -93,7 +93,7 @@ static struct machine *setup_fake_machine(struct machines *machines)
if (thread == NULL)
goto out;
- thread__set_comm(thread, fake_threads[i].comm);
+ thread__set_comm(thread, fake_threads[i].comm, 0);
}
for (i = 0; i < ARRAY_SIZE(fake_mmap_info); i++) {
@@ -110,7 +110,7 @@ static struct machine *setup_fake_machine(struct machines *machines)
strcpy(fake_mmap_event.mmap.filename,
fake_mmap_info[i].filename);
- machine__process_mmap_event(machine, &fake_mmap_event);
+ machine__process_mmap_event(machine, &fake_mmap_event, NULL);
}
for (i = 0; i < ARRAY_SIZE(fake_symbols); i++) {
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index d639bfe29309..c7b2458b47a6 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -516,18 +516,18 @@ size_t perf_event__fprintf_comm(union perf_event *event, FILE *fp)
int perf_event__process_comm(struct perf_tool *tool __maybe_unused,
union perf_event *event,
- struct perf_sample *sample __maybe_unused,
+ struct perf_sample *sample,
struct machine *machine)
{
- return machine__process_comm_event(machine, event);
+ return machine__process_comm_event(machine, event, sample);
}
int perf_event__process_lost(struct perf_tool *tool __maybe_unused,
union perf_event *event,
- struct perf_sample *sample __maybe_unused,
+ struct perf_sample *sample,
struct machine *machine)
{
- return machine__process_lost_event(machine, event);
+ return machine__process_lost_event(machine, event, sample);
}
size_t perf_event__fprintf_mmap(union perf_event *event, FILE *fp)
@@ -550,18 +550,18 @@ size_t perf_event__fprintf_mmap2(union perf_event *event, FILE *fp)
int perf_event__process_mmap(struct perf_tool *tool __maybe_unused,
union perf_event *event,
- struct perf_sample *sample __maybe_unused,
+ struct perf_sample *sample,
struct machine *machine)
{
- return machine__process_mmap_event(machine, event);
+ return machine__process_mmap_event(machine, event, sample);
}
int perf_event__process_mmap2(struct perf_tool *tool __maybe_unused,
union perf_event *event,
- struct perf_sample *sample __maybe_unused,
+ struct perf_sample *sample,
struct machine *machine)
{
- return machine__process_mmap2_event(machine, event);
+ return machine__process_mmap2_event(machine, event, sample);
}
size_t perf_event__fprintf_task(union perf_event *event, FILE *fp)
@@ -573,18 +573,18 @@ size_t perf_event__fprintf_task(union perf_event *event, FILE *fp)
int perf_event__process_fork(struct perf_tool *tool __maybe_unused,
union perf_event *event,
- struct perf_sample *sample __maybe_unused,
+ struct perf_sample *sample,
struct machine *machine)
{
- return machine__process_fork_event(machine, event);
+ return machine__process_fork_event(machine, event, sample);
}
int perf_event__process_exit(struct perf_tool *tool __maybe_unused,
union perf_event *event,
- struct perf_sample *sample __maybe_unused,
+ struct perf_sample *sample,
struct machine *machine)
{
- return machine__process_exit_event(machine, event);
+ return machine__process_exit_event(machine, event, sample);
}
size_t perf_event__fprintf(union perf_event *event, FILE *fp)
@@ -615,10 +615,10 @@ size_t perf_event__fprintf(union perf_event *event, FILE *fp)
int perf_event__process(struct perf_tool *tool __maybe_unused,
union perf_event *event,
- struct perf_sample *sample __maybe_unused,
+ struct perf_sample *sample,
struct machine *machine)
{
- return machine__process_event(machine, event);
+ return machine__process_event(machine, event, sample);
}
void thread__find_addr_map(struct thread *self,
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 933d14f287ca..b8fc8a0d03ec 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -40,7 +40,7 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
return -ENOMEM;
snprintf(comm, sizeof(comm), "[guest/%d]", pid);
- thread__set_comm(thread, comm);
+ thread__set_comm(thread, comm, 0);
}
return 0;
@@ -314,7 +314,8 @@ struct thread *machine__find_thread(struct machine *machine, pid_t tid)
return __machine__findnew_thread(machine, 0, tid, false);
}
-int machine__process_comm_event(struct machine *machine, union perf_event *event)
+int machine__process_comm_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample)
{
struct thread *thread = machine__findnew_thread(machine,
event->comm.pid,
@@ -323,7 +324,7 @@ int machine__process_comm_event(struct machine *machine, union perf_event *event
if (dump_trace)
perf_event__fprintf_comm(event, stdout);
- if (thread == NULL || thread__set_comm(thread, event->comm.comm)) {
+ if (thread == NULL || thread__set_comm(thread, event->comm.comm, sample->time)) {
dump_printf("problem processing PERF_RECORD_COMM, skipping event.\n");
return -1;
}
@@ -332,7 +333,7 @@ int machine__process_comm_event(struct machine *machine, union perf_event *event
}
int machine__process_lost_event(struct machine *machine __maybe_unused,
- union perf_event *event)
+ union perf_event *event, struct perf_sample *sample __maybe_unused)
{
dump_printf(": id:%" PRIu64 ": lost:%" PRIu64 "\n",
event->lost.id, event->lost.lost);
@@ -998,7 +999,8 @@ out_problem:
}
int machine__process_mmap2_event(struct machine *machine,
- union perf_event *event)
+ union perf_event *event,
+ struct perf_sample *sample __maybe_unused)
{
u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
struct thread *thread;
@@ -1045,7 +1047,8 @@ out_problem:
return 0;
}
-int machine__process_mmap_event(struct machine *machine, union perf_event *event)
+int machine__process_mmap_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample __maybe_unused)
{
u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
struct thread *thread;
@@ -1102,7 +1105,8 @@ static void machine__remove_thread(struct machine *machine, struct thread *th)
list_add_tail(&th->node, &machine->dead_threads);
}
-int machine__process_fork_event(struct machine *machine, union perf_event *event)
+int machine__process_fork_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample)
{
struct thread *thread = machine__find_thread(machine, event->fork.tid);
struct thread *parent = machine__findnew_thread(machine,
@@ -1119,7 +1123,7 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
perf_event__fprintf_task(event, stdout);
if (thread == NULL || parent == NULL ||
- thread__fork(thread, parent) < 0) {
+ thread__fork(thread, parent, sample->time) < 0) {
dump_printf("problem processing PERF_RECORD_FORK, skipping event.\n");
return -1;
}
@@ -1127,8 +1131,8 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
return 0;
}
-int machine__process_exit_event(struct machine *machine __maybe_unused,
- union perf_event *event)
+int machine__process_exit_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample __maybe_unused)
{
struct thread *thread = machine__find_thread(machine, event->fork.tid);
@@ -1141,23 +1145,24 @@ int machine__process_exit_event(struct machine *machine __maybe_unused,
return 0;
}
-int machine__process_event(struct machine *machine, union perf_event *event)
+int machine__process_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample)
{
int ret;
switch (event->header.type) {
case PERF_RECORD_COMM:
- ret = machine__process_comm_event(machine, event); break;
+ ret = machine__process_comm_event(machine, event, sample); break;
case PERF_RECORD_MMAP:
- ret = machine__process_mmap_event(machine, event); break;
+ ret = machine__process_mmap_event(machine, event, sample); break;
case PERF_RECORD_MMAP2:
- ret = machine__process_mmap2_event(machine, event); break;
+ ret = machine__process_mmap2_event(machine, event, sample); break;
case PERF_RECORD_FORK:
- ret = machine__process_fork_event(machine, event); break;
+ ret = machine__process_fork_event(machine, event, sample); break;
case PERF_RECORD_EXIT:
- ret = machine__process_exit_event(machine, event); break;
+ ret = machine__process_exit_event(machine, event, sample); break;
case PERF_RECORD_LOST:
- ret = machine__process_lost_event(machine, event); break;
+ ret = machine__process_lost_event(machine, event, sample); break;
default:
ret = -1;
break;
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 58a6be1fc739..1db29d8ce595 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -40,13 +40,20 @@ struct map *machine__kernel_map(struct machine *machine, enum map_type type)
struct thread *machine__find_thread(struct machine *machine, pid_t tid);
-int machine__process_comm_event(struct machine *machine, union perf_event *event);
-int machine__process_exit_event(struct machine *machine, union perf_event *event);
-int machine__process_fork_event(struct machine *machine, union perf_event *event);
-int machine__process_lost_event(struct machine *machine, union perf_event *event);
-int machine__process_mmap_event(struct machine *machine, union perf_event *event);
-int machine__process_mmap2_event(struct machine *machine, union perf_event *event);
-int machine__process_event(struct machine *machine, union perf_event *event);
+int machine__process_comm_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample);
+int machine__process_exit_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample);
+int machine__process_fork_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample);
+int machine__process_lost_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample);
+int machine__process_mmap_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample);
+int machine__process_mmap2_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample);
+int machine__process_event(struct machine *machine, union perf_event *event,
+ struct perf_sample *sample);
typedef void (*machine__process_t)(struct machine *machine, void *data);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 9f62be8bc167..8de3ab388187 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1130,7 +1130,7 @@ static struct thread *perf_session__register_idle_thread(struct perf_session *se
{
struct thread *thread = perf_session__findnew(self, 0);
- if (thread == NULL || thread__set_comm(thread, "swapper")) {
+ if (thread == NULL || thread__set_comm(thread, "swapper", 0)) {
pr_err("problem inserting idle task.\n");
thread = NULL;
}
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index bb50b9fda9c2..509abed750e6 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -31,7 +31,8 @@ void thread__delete(struct thread *self)
free(self);
}
-int thread__set_comm(struct thread *self, const char *comm)
+int thread__set_comm(struct thread *self, const char *comm,
+ u64 timestamp __maybe_unused)
{
int err;
@@ -73,7 +74,8 @@ void thread__insert_map(struct thread *self, struct map *map)
map_groups__insert(&self->mg, map);
}
-int thread__fork(struct thread *self, struct thread *parent)
+int thread__fork(struct thread *self, struct thread *parent,
+ u64 timestamp __maybe_unused)
{
int i;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 84db214d90bd..b738c9948508 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -33,11 +33,11 @@ static inline void thread__exited(struct thread *thread)
thread->dead = true;
}
-int thread__set_comm(struct thread *self, const char *comm);
+int thread__set_comm(struct thread *self, const char *comm, u64 timestamp);
int thread__comm_len(struct thread *self);
const char *thread__comm_curr(const struct thread *thread);
void thread__insert_map(struct thread *self, struct map *map);
-int thread__fork(struct thread *self, struct thread *parent);
+int thread__fork(struct thread *self, struct thread *parent, u64 timestamp);
size_t thread__fprintf(struct thread *thread, FILE *fp);
static inline struct map *thread__find_map(struct thread *self,
--
1.7.11.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 6/8] perf tools: Add new comm infrastructure
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
` (4 preceding siblings ...)
2013-09-26 8:58 ` [PATCH 5/8] perf tools: Add time argument on comm setting Namhyung Kim
@ 2013-09-26 8:58 ` Namhyung Kim
2013-09-26 8:58 ` [PATCH 7/8] perf tools: Compare hists comm by addresses Namhyung Kim
` (3 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-09-26 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Linus Torvalds, Frederic Weisbecker, Jiri Olsa, David Ahern,
Ingo Molnar, Arnaldo Carvalho de Melo, Stephane Eranian
From: Frederic Weisbecker <fweisbec@gmail.com>
This new comm infrastructure provides two features:
1) It keeps track of all comms lifecycle for a given thread. This
way we can associate a timeframe to any thread comm, as long as
PERF_SAMPLE_TIME samples are joined to comm and fork events.
As a result we should have more precise comm sorted hists with
seperated entries for pre and post exec time after a fork.
2) It also makes sure that a given comm string is not duplicated
but rather shared among the threads that refer to it. This way the
threads comm can be compared against pointer values from the sort
infrastructure.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-hwjf70b2wve9m2kosxiq8bb3@git.kernel.org
[ Fixed up minor const pointer issues and removed 'self' usage ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Makefile | 2 +
tools/perf/util/comm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/comm.h | 20 +++++++++
tools/perf/util/thread.c | 100 +++++++++++++++++++++++++++++++-------------
tools/perf/util/thread.h | 3 +-
5 files changed, 202 insertions(+), 29 deletions(-)
create mode 100644 tools/perf/util/comm.c
create mode 100644 tools/perf/util/comm.h
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index b62e12d2ba6b..42001f387e7f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -272,6 +272,7 @@ LIB_H += util/color.h
LIB_H += util/values.h
LIB_H += util/sort.h
LIB_H += util/hist.h
+LIB_H += util/comm.h
LIB_H += util/thread.h
LIB_H += util/thread_map.h
LIB_H += util/trace-event.h
@@ -339,6 +340,7 @@ LIB_OBJS += $(OUTPUT)util/machine.o
LIB_OBJS += $(OUTPUT)util/map.o
LIB_OBJS += $(OUTPUT)util/pstack.o
LIB_OBJS += $(OUTPUT)util/session.o
+LIB_OBJS += $(OUTPUT)util/comm.o
LIB_OBJS += $(OUTPUT)util/thread.o
LIB_OBJS += $(OUTPUT)util/thread_map.o
LIB_OBJS += $(OUTPUT)util/trace-event-parse.o
diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
new file mode 100644
index 000000000000..0cfb55202537
--- /dev/null
+++ b/tools/perf/util/comm.c
@@ -0,0 +1,106 @@
+#include "comm.h"
+#include "util.h"
+#include <stdlib.h>
+#include <stdio.h>
+
+struct comm_str {
+ char *str;
+ struct rb_node rb_node;
+ int ref;
+};
+
+/* Should perhaps be moved to struct machine */
+static struct rb_root comm_str_root;
+
+static void comm_str_get(struct comm_str *cs)
+{
+ cs->ref++;
+}
+
+static void comm_str_put(struct comm_str *cs)
+{
+ if (!--cs->ref) {
+ rb_erase(&cs->rb_node, &comm_str_root);
+ free(cs->str);
+ free(cs);
+ }
+}
+
+static struct comm_str *comm_str_alloc(const char *str)
+{
+ struct comm_str *cs;
+
+ cs = zalloc(sizeof(*cs));
+ if (!cs)
+ return NULL;
+
+ cs->str = strdup(str);
+ if (!cs->str) {
+ free(cs);
+ return NULL;
+ }
+
+ return cs;
+}
+
+static struct comm_str *comm_str_findnew(const char *str, struct rb_root *root)
+{
+ struct rb_node **p = &root->rb_node;
+ struct rb_node *parent = NULL;
+ struct comm_str *iter, *new;
+ int cmp;
+
+ while (*p != NULL) {
+ parent = *p;
+ iter = rb_entry(parent, struct comm_str, rb_node);
+
+ cmp = strcmp(str, iter->str);
+ if (!cmp)
+ return iter;
+
+ if (cmp < 0)
+ p = &(*p)->rb_left;
+ else
+ p = &(*p)->rb_right;
+ }
+
+ new = comm_str_alloc(str);
+ if (!new)
+ return NULL;
+
+ rb_link_node(&new->rb_node, parent, p);
+ rb_insert_color(&new->rb_node, root);
+
+ return new;
+}
+
+struct comm *comm__new(const char *str, u64 timestamp)
+{
+ struct comm *comm = zalloc(sizeof(*comm));
+
+ if (!comm)
+ return NULL;
+
+ comm->start = timestamp;
+
+ comm->comm_str = comm_str_findnew(str, &comm_str_root);
+ if (!comm->comm_str) {
+ free(comm);
+ return NULL;
+ }
+
+ comm_str_get(comm->comm_str);
+
+ return comm;
+}
+
+void comm__free(struct comm *comm)
+{
+ comm_str_put(comm->comm_str);
+ free(comm);
+}
+
+const char *comm__str(const struct comm *comm)
+{
+ return comm->comm_str->str;
+}
diff --git a/tools/perf/util/comm.h b/tools/perf/util/comm.h
new file mode 100644
index 000000000000..f62d215bede2
--- /dev/null
+++ b/tools/perf/util/comm.h
@@ -0,0 +1,20 @@
+#ifndef __PERF_COMM_H
+#define __PERF_COMM_H
+
+#include "../perf.h"
+#include <linux/rbtree.h>
+#include <linux/list.h>
+
+struct comm_str;
+
+struct comm {
+ struct comm_str *comm_str;
+ u64 start;
+ struct list_head list;
+};
+
+void comm__free(struct comm *comm);
+struct comm *comm__new(const char *str, u64 timestamp);
+const char *comm__str(const struct comm *comm);
+
+#endif /* __PERF_COMM_H */
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 509abed750e6..9cb01566eff1 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -6,9 +6,12 @@
#include "thread.h"
#include "util.h"
#include "debug.h"
+#include "comm.h"
struct thread *thread__new(pid_t pid, pid_t tid)
{
+ char *comm_str;
+ struct comm *comm;
struct thread *self = zalloc(sizeof(*self));
if (self != NULL) {
@@ -16,47 +19,88 @@ struct thread *thread__new(pid_t pid, pid_t tid)
self->pid_ = pid;
self->tid = tid;
self->ppid = -1;
- self->comm = malloc(32);
- if (self->comm)
- snprintf(self->comm, 32, ":%d", self->tid);
+ INIT_LIST_HEAD(&self->comm_list);
+
+ comm_str = malloc(32);
+ if (!comm_str)
+ goto err_thread;
+
+ snprintf(comm_str, 32, ":%d", tid);
+ comm = comm__new(comm_str, 0);
+ free(comm_str);
+ if (!comm)
+ goto err_thread;
+
+ list_add(&comm->list, &self->comm_list);
}
return self;
+
+err_thread:
+ free(self);
+ return NULL;
}
void thread__delete(struct thread *self)
{
+ struct comm *comm, *tmp;
+
map_groups__exit(&self->mg);
- free(self->comm);
+ list_for_each_entry_safe(comm, tmp, &self->comm_list, list) {
+ list_del(&comm->list);
+ comm__free(comm);
+ }
+
free(self);
}
-int thread__set_comm(struct thread *self, const char *comm,
- u64 timestamp __maybe_unused)
+static struct comm *curr_comm(const struct thread *thread)
{
- int err;
-
- if (self->comm)
- free(self->comm);
- self->comm = strdup(comm);
- err = self->comm == NULL ? -ENOMEM : 0;
- if (!err) {
- self->comm_set = true;
+ if (list_empty(&thread->comm_list))
+ return NULL;
+
+ return list_first_entry(&thread->comm_list, struct comm, list);
+}
+
+/* CHECKME: time should always be 0 if event aren't ordered */
+int thread__set_comm(struct thread *thread, const char *str, u64 timestamp)
+{
+ struct comm *new, *curr = curr_comm(thread);
+
+ /* Override latest entry if it had no specific time coverage */
+ if (!curr->start) {
+ list_del(&curr->list);
+ comm__free(curr);
}
- return err;
+
+ new = comm__new(str, timestamp);
+ if (!new)
+ return -ENOMEM;
+
+ list_add(&new->list, &thread->comm_list);
+ thread->comm_set = true;
+
+ return 0;
}
const char *thread__comm_curr(const struct thread *thread)
{
- return thread->comm;
+ const struct comm *comm = curr_comm(thread);
+
+ if (!comm)
+ return NULL;
+
+ return comm__str(comm);
}
+/* CHECKME: it should probably better return the max comm len from its comm list */
int thread__comm_len(struct thread *self)
{
if (!self->comm_len) {
- if (!self->comm)
+ const char *comm = thread__comm_curr(self);
+ if (!comm)
return 0;
- self->comm_len = strlen(self->comm);
+ self->comm_len = strlen(comm);
}
return self->comm_len;
@@ -74,25 +118,25 @@ void thread__insert_map(struct thread *self, struct map *map)
map_groups__insert(&self->mg, map);
}
-int thread__fork(struct thread *self, struct thread *parent,
- u64 timestamp __maybe_unused)
+int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
{
- int i;
+ int i, err;
if (parent->comm_set) {
- if (self->comm)
- free(self->comm);
- self->comm = strdup(parent->comm);
- if (!self->comm)
+ const char *comm = thread__comm_curr(parent);
+ if (!comm)
return -ENOMEM;
- self->comm_set = true;
+ err = thread__set_comm(thread, comm, timestamp);
+ if (!err)
+ return err;
+ thread->comm_set = true;
}
for (i = 0; i < MAP__NR_TYPES; ++i)
- if (map_groups__clone(&self->mg, &parent->mg, i) < 0)
+ if (map_groups__clone(&thread->mg, &parent->mg, i) < 0)
return -ENOMEM;
- self->ppid = parent->tid;
+ thread->ppid = parent->tid;
return 0;
}
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index b738c9948508..0ec37bae88d0 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -2,6 +2,7 @@
#define __PERF_THREAD_H
#include <linux/rbtree.h>
+#include <linux/list.h>
#include <unistd.h>
#include <sys/types.h>
#include "symbol.h"
@@ -18,7 +19,7 @@ struct thread {
char shortname[3];
bool comm_set;
bool dead; /* if set thread has exited */
- char *comm;
+ struct list_head comm_list;
int comm_len;
void *priv;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 7/8] perf tools: Compare hists comm by addresses
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
` (5 preceding siblings ...)
2013-09-26 8:58 ` [PATCH 6/8] perf tools: Add new comm infrastructure Namhyung Kim
@ 2013-09-26 8:58 ` Namhyung Kim
2013-09-26 8:58 ` [PATCH 8/8] perf tools: Get current comm instead of last one Namhyung Kim
` (2 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-09-26 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Linus Torvalds, Frederic Weisbecker, Jiri Olsa, David Ahern,
Ingo Molnar, Arnaldo Carvalho de Melo, Stephane Eranian
From: Frederic Weisbecker <fweisbec@gmail.com>
Now that comm strings are allocated only once and refcounted to be shared
among threads, these can now be safely compared by addresses. This
should remove most hists collapses on post processing.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/sort.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 65fe958ce30b..48e6a38132c9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -79,7 +79,8 @@ struct sort_entry sort_thread = {
static int64_t
sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
{
- return right->thread->tid - left->thread->tid;
+ /* Compare the addr that should be unique among comm */
+ return thread__comm_curr(right->thread) - thread__comm_curr(left->thread);
}
static int64_t
--
1.7.11.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 8/8] perf tools: Get current comm instead of last one
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
` (6 preceding siblings ...)
2013-09-26 8:58 ` [PATCH 7/8] perf tools: Compare hists comm by addresses Namhyung Kim
@ 2013-09-26 8:58 ` Namhyung Kim
2013-10-02 10:01 ` Frederic Weisbecker
2013-09-26 9:34 ` [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Ingo Molnar
2013-09-26 13:46 ` David Ahern
9 siblings, 1 reply; 24+ messages in thread
From: Namhyung Kim @ 2013-09-26 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Linus Torvalds, Frederic Weisbecker, Jiri Olsa,
Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung.kim@lge.com>
At insert time, a hist entry should reference comm at the time
otherwise it'll get the last comm anyway.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Link: http://lkml.kernel.org/n/tip-n6pykiiymtgmcjs834go2t8x@git.kernel.org
[ Fixed up const pointer issues ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/comm.c | 15 +++++++++++++++
tools/perf/util/comm.h | 1 +
tools/perf/util/hist.c | 3 +++
tools/perf/util/sort.c | 14 +++++---------
tools/perf/util/sort.h | 1 +
tools/perf/util/thread.c | 6 +++---
tools/perf/util/thread.h | 2 ++
7 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
index 0cfb55202537..ecd3c9938d3e 100644
--- a/tools/perf/util/comm.c
+++ b/tools/perf/util/comm.c
@@ -94,6 +94,21 @@ struct comm *comm__new(const char *str, u64 timestamp)
return comm;
}
+void comm__override(struct comm *comm, const char *str, u64 timestamp)
+{
+ struct comm_str *old = comm->comm_str;
+
+ comm->comm_str = comm_str_findnew(str, &comm_str_root);
+ if (!comm->comm_str) {
+ comm->comm_str = old;
+ return;
+ }
+
+ comm->start = timestamp;
+ comm_str_get(comm->comm_str);
+ comm_str_put(old);
+}
+
void comm__free(struct comm *comm)
{
comm_str_put(comm->comm_str);
diff --git a/tools/perf/util/comm.h b/tools/perf/util/comm.h
index f62d215bede2..b2c364b37468 100644
--- a/tools/perf/util/comm.h
+++ b/tools/perf/util/comm.h
@@ -16,5 +16,6 @@ struct comm {
void comm__free(struct comm *comm);
struct comm *comm__new(const char *str, u64 timestamp);
const char *comm__str(const struct comm *comm);
+void comm__override(struct comm *self, const char *str, u64 timestamp);
#endif /* __PERF_COMM_H */
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index a2d58e7abdcf..1aa97b012a09 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -412,6 +412,7 @@ struct hist_entry *__hists__add_mem_entry(struct hists *self,
{
struct hist_entry entry = {
.thread = al->thread,
+ .comm = curr_comm(al->thread),
.ms = {
.map = al->map,
.sym = al->sym,
@@ -442,6 +443,7 @@ struct hist_entry *__hists__add_branch_entry(struct hists *self,
{
struct hist_entry entry = {
.thread = al->thread,
+ .comm = curr_comm(al->thread),
.ms = {
.map = bi->to.map,
.sym = bi->to.sym,
@@ -471,6 +473,7 @@ struct hist_entry *__hists__add_entry(struct hists *self,
{
struct hist_entry entry = {
.thread = al->thread,
+ .comm = curr_comm(al->thread),
.ms = {
.map = al->map,
.sym = al->sym,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 48e6a38132c9..eefc0e623966 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1,5 +1,6 @@
#include "sort.h"
#include "hist.h"
+#include "comm.h"
#include "symbol.h"
regex_t parent_regex;
@@ -80,25 +81,20 @@ static int64_t
sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
{
/* Compare the addr that should be unique among comm */
- return thread__comm_curr(right->thread) - thread__comm_curr(left->thread);
+ return comm__str(right->comm) - comm__str(left->comm);
}
static int64_t
sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
{
- const char *comm_l = thread__comm_curr(left->thread);
- const char *comm_r = thread__comm_curr(right->thread);
-
- if (!comm_l || !comm_r)
- return cmp_null(comm_l, comm_r);
-
- return strcmp(comm_l, comm_r);
+ /* Compare the addr that should be unique among comm */
+ return comm__str(right->comm) - comm__str(left->comm);
}
static int hist_entry__comm_snprintf(struct hist_entry *self, char *bf,
size_t size, unsigned int width)
{
- return repsep_snprintf(bf, size, "%*s", width, thread__comm_curr(self->thread));
+ return repsep_snprintf(bf, size, "%*s", width, comm__str(self->comm));
}
struct sort_entry sort_comm = {
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 4e80dbd271e7..4d0153f83e3c 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -84,6 +84,7 @@ struct hist_entry {
struct he_stat stat;
struct map_symbol ms;
struct thread *thread;
+ struct comm *comm;
u64 ip;
s32 cpu;
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 9cb01566eff1..444de7a8527f 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -54,7 +54,7 @@ void thread__delete(struct thread *self)
free(self);
}
-static struct comm *curr_comm(const struct thread *thread)
+struct comm *curr_comm(const struct thread *thread)
{
if (list_empty(&thread->comm_list))
return NULL;
@@ -69,8 +69,8 @@ int thread__set_comm(struct thread *thread, const char *str, u64 timestamp)
/* Override latest entry if it had no specific time coverage */
if (!curr->start) {
- list_del(&curr->list);
- comm__free(curr);
+ comm__override(curr, str, timestamp);
+ return 0;
}
new = comm__new(str, timestamp);
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 0ec37bae88d0..196c6c2dfbe1 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -26,6 +26,7 @@ struct thread {
};
struct machine;
+struct comm;
struct thread *thread__new(pid_t pid, pid_t tid);
void thread__delete(struct thread *self);
@@ -36,6 +37,7 @@ static inline void thread__exited(struct thread *thread)
int thread__set_comm(struct thread *self, const char *comm, u64 timestamp);
int thread__comm_len(struct thread *self);
+struct comm *curr_comm(const struct thread *self);
const char *thread__comm_curr(const struct thread *thread);
void thread__insert_map(struct thread *self, struct map *map);
int thread__fork(struct thread *self, struct thread *parent, u64 timestamp);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 8/8] perf tools: Get current comm instead of last one
2013-09-26 8:58 ` [PATCH 8/8] perf tools: Get current comm instead of last one Namhyung Kim
@ 2013-10-02 10:01 ` Frederic Weisbecker
2013-10-08 1:56 ` Namhyung Kim
0 siblings, 1 reply; 24+ messages in thread
From: Frederic Weisbecker @ 2013-10-02 10:01 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Linus Torvalds, Jiri Olsa,
Arnaldo Carvalho de Melo
On Thu, Sep 26, 2013 at 05:58:10PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
>
> At insert time, a hist entry should reference comm at the time
> otherwise it'll get the last comm anyway.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Link: http://lkml.kernel.org/n/tip-n6pykiiymtgmcjs834go2t8x@git.kernel.org
> [ Fixed up const pointer issues ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> tools/perf/util/comm.c | 15 +++++++++++++++
> tools/perf/util/comm.h | 1 +
> tools/perf/util/hist.c | 3 +++
> tools/perf/util/sort.c | 14 +++++---------
> tools/perf/util/sort.h | 1 +
> tools/perf/util/thread.c | 6 +++---
> tools/perf/util/thread.h | 2 ++
> 7 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
> index 0cfb55202537..ecd3c9938d3e 100644
> --- a/tools/perf/util/comm.c
> +++ b/tools/perf/util/comm.c
> @@ -94,6 +94,21 @@ struct comm *comm__new(const char *str, u64 timestamp)
> return comm;
> }
>
> +void comm__override(struct comm *comm, const char *str, u64 timestamp)
> +{
> + struct comm_str *old = comm->comm_str;
> +
> + comm->comm_str = comm_str_findnew(str, &comm_str_root);
> + if (!comm->comm_str) {
> + comm->comm_str = old;
> + return;
> + }
> +
> + comm->start = timestamp;
> + comm_str_get(comm->comm_str);
> + comm_str_put(old);
> +}
> +
> void comm__free(struct comm *comm)
> {
> comm_str_put(comm->comm_str);
> diff --git a/tools/perf/util/comm.h b/tools/perf/util/comm.h
> index f62d215bede2..b2c364b37468 100644
> --- a/tools/perf/util/comm.h
> +++ b/tools/perf/util/comm.h
> @@ -16,5 +16,6 @@ struct comm {
> void comm__free(struct comm *comm);
> struct comm *comm__new(const char *str, u64 timestamp);
> const char *comm__str(const struct comm *comm);
> +void comm__override(struct comm *self, const char *str, u64 timestamp);
>
> #endif /* __PERF_COMM_H */
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index a2d58e7abdcf..1aa97b012a09 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -412,6 +412,7 @@ struct hist_entry *__hists__add_mem_entry(struct hists *self,
> {
> struct hist_entry entry = {
> .thread = al->thread,
> + .comm = curr_comm(al->thread),
So now that we have an accessor for that, it clashes a bit with existing names.
I suggest we rename thread__comm_curr() -> thread__comm_str()
And rename curr_comm() -> thread__comm()
Hmm?
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 8/8] perf tools: Get current comm instead of last one
2013-10-02 10:01 ` Frederic Weisbecker
@ 2013-10-08 1:56 ` Namhyung Kim
0 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-10-08 1:56 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Linus Torvalds, Jiri Olsa,
Arnaldo Carvalho de Melo
Hi Frederic,
On Wed, 2 Oct 2013 12:01:51 +0200, Frederic Weisbecker wrote:
> On Thu, Sep 26, 2013 at 05:58:10PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim <namhyung.kim@lge.com>
>>
>> At insert time, a hist entry should reference comm at the time
>> otherwise it'll get the last comm anyway.
[SNIP]
>> --- a/tools/perf/util/comm.h
>> +++ b/tools/perf/util/comm.h
>> @@ -16,5 +16,6 @@ struct comm {
>> void comm__free(struct comm *comm);
>> struct comm *comm__new(const char *str, u64 timestamp);
>> const char *comm__str(const struct comm *comm);
>> +void comm__override(struct comm *self, const char *str, u64 timestamp);
>>
>> #endif /* __PERF_COMM_H */
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index a2d58e7abdcf..1aa97b012a09 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -412,6 +412,7 @@ struct hist_entry *__hists__add_mem_entry(struct hists *self,
>> {
>> struct hist_entry entry = {
>> .thread = al->thread,
>> + .comm = curr_comm(al->thread),
>
> So now that we have an accessor for that, it clashes a bit with existing names.
>
> I suggest we rename thread__comm_curr() -> thread__comm_str()
> And rename curr_comm() -> thread__comm()
>
> Hmm?
Okay, I'll rename and resend.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
` (7 preceding siblings ...)
2013-09-26 8:58 ` [PATCH 8/8] perf tools: Get current comm instead of last one Namhyung Kim
@ 2013-09-26 9:34 ` Ingo Molnar
2013-09-27 2:08 ` Namhyung Kim
2013-09-26 13:46 ` David Ahern
9 siblings, 1 reply; 24+ messages in thread
From: Ingo Molnar @ 2013-09-26 9:34 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Namhyung Kim, LKML, Linus Torvalds, Frederic Weisbecker,
Jiri Olsa
* Namhyung Kim <namhyung@kernel.org> wrote:
> Hello,
>
> This is a new version of callchain improvement patchset. I found and
> fixed bugs in the previous version. I verified that it produced
> exactly same output before and after applying rbtree conversion patch
> (#1). However after Frederic's new comm infrastructure patches are
> applied it'd be little different.
>
> The patches are on 'perf/callchain-v4' branch in my tree
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
>
> Please test!
>
> Thanks,
> Namhyung
>
>
> Frederic Weisbecker (4):
> perf tools: Use an accessor to read thread comm
> perf tools: Add time argument on comm setting
> perf tools: Add new comm infrastructure
> perf tools: Compare hists comm by addresses
>
> Namhyung Kim (4):
> perf callchain: Convert children list to rbtree
> perf ui/progress: Add new helper functions for progress bar
> perf tools: Show progress on histogram collapsing
> perf tools: Get current comm instead of last one
> 31 files changed, 504 insertions(+), 177 deletions(-)
I pulled this into a test-branch and resolved the builtin-trace.c conflict
with tip:master.
I tried your new code out with Linus's testcase of a parallel kernel
build:
perf record -g -- make -j8 bzImage
The 'perf record' session went mostly fine except for lost events:
Kernel: arch/x86/boot/bzImage is ready (#25)
[ perf record: Woken up 553 times to write data ]
[ perf record: Captured and wrote 196.811 MB perf.data (~8598789 samples) ]
Warning:
Processed 2631982 events and lost 54 chunks!
Check IO/CPU overload!
So, for completeness I'll mention that perf fails in two ways here:
- UI bug: it prints some scary warnings about 'IO/CPU overload' but
does not give any idea what the user could do about it. At minimum it
should print something about increasing --mmap-pages. It should also
print the current value of --mmap-pages so that the user knows to what
value to increase it ...
- defaults bug: this isn't an extreme workload on an extreme box - it's
a relatively bog standard system with 8 cores and 16 CPUs. The kernel
build was mild as well, with -j8. So losing events in perf record is
absolutely unacceptable. A solution might be to automatically increase
the ring-buffer if -g is used, in expectation of a higher data rate.
Once perf record was done I had the ~200 MB perf.data - and the 'perf
report' session went much smoother than ever before: the analysis went
very quickly, it finished within 10 seconds and displayed a nice progress
bar. So this is entirely usable IMO.
One small 'perf report' annoyance is that during the analysis passes
missing symbol printouts flash in the TUI - without the user having a
chance to read them. So those messages should either linger, or be
displayed at a later stage, for example when the user is confronted with
non-resolved symbols like:
+ 28.63% cc1 cc1 [.] 0x00000000006a92cb ◆
that is the point where the message would be useful - but nothing
indicates at all that this is an undesirable symbol entry and nothing
helps the user what to do about it!
A solution might be to display non-intrusive messages about non-resolved
symbols when such a symbol is manipulated (its children are openened, or
annotation is attempted).
Here there is a second annoyance: on the detailed screen the 'annotate'
entry is simply missing when the symbol has not been resolved. If I hit
'a' on the symbol entry itself in the graph view, then sometimes
annotation works - sometimes it does not and there's no UI feedback
whatsoever why it's not working.
I'm not suggesting to change the keyboard flow - that is very smooth - but
display information about failed annotations in the status line at the
bottom of the screen would be very useful. Remember: it's _always_ an UI
bug if the user hits a key and absolutely nothing happens. At minimum a
low-key, non-intrusive 'key X not bound' message should appear in the
status line bottom. Deterministic action/reaction sequences are utterly
important when interacting with computers.
Anyway, very nice progress with the tree here!
Thanks,
Ingo
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)
2013-09-26 9:34 ` [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Ingo Molnar
@ 2013-09-27 2:08 ` Namhyung Kim
0 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-09-27 2:08 UTC (permalink / raw)
To: Ingo Molnar
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Namhyung Kim, LKML, Linus Torvalds, Frederic Weisbecker,
Jiri Olsa
Hi Ingo,
On Thu, 26 Sep 2013 11:34:26 +0200, Ingo Molnar wrote:
> * Namhyung Kim <namhyung@kernel.org> wrote:
>
>> Hello,
>>
>> This is a new version of callchain improvement patchset. I found and
>> fixed bugs in the previous version. I verified that it produced
>> exactly same output before and after applying rbtree conversion patch
>> (#1). However after Frederic's new comm infrastructure patches are
>> applied it'd be little different.
>>
>> The patches are on 'perf/callchain-v4' branch in my tree
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> I pulled this into a test-branch and resolved the builtin-trace.c conflict
> with tip:master.
>
> I tried your new code out with Linus's testcase of a parallel kernel
> build:
>
> perf record -g -- make -j8 bzImage
>
> The 'perf record' session went mostly fine except for lost events:
>
> Kernel: arch/x86/boot/bzImage is ready (#25)
> [ perf record: Woken up 553 times to write data ]
> [ perf record: Captured and wrote 196.811 MB perf.data (~8598789 samples) ]
> Warning:
> Processed 2631982 events and lost 54 chunks!
>
> Check IO/CPU overload!
>
> So, for completeness I'll mention that perf fails in two ways here:
>
> - UI bug: it prints some scary warnings about 'IO/CPU overload' but
> does not give any idea what the user could do about it. At minimum it
> should print something about increasing --mmap-pages. It should also
> print the current value of --mmap-pages so that the user knows to what
> value to increase it ...
>
> - defaults bug: this isn't an extreme workload on an extreme box - it's
> a relatively bog standard system with 8 cores and 16 CPUs. The kernel
> build was mild as well, with -j8. So losing events in perf record is
> absolutely unacceptable. A solution might be to automatically increase
> the ring-buffer if -g is used, in expectation of a higher data rate.
Both look sensible. I'll try to cook patches for them.
>
> Once perf record was done I had the ~200 MB perf.data - and the 'perf
> report' session went much smoother than ever before: the analysis went
> very quickly, it finished within 10 seconds and displayed a nice progress
> bar. So this is entirely usable IMO.
>
> One small 'perf report' annoyance is that during the analysis passes
> missing symbol printouts flash in the TUI - without the user having a
> chance to read them. So those messages should either linger, or be
> displayed at a later stage, for example when the user is confronted with
> non-resolved symbols like:
>
> + 28.63% cc1 cc1 [.] 0x00000000006a92cb ◆
>
> that is the point where the message would be useful - but nothing
> indicates at all that this is an undesirable symbol entry and nothing
> helps the user what to do about it!
>
> A solution might be to display non-intrusive messages about non-resolved
> symbols when such a symbol is manipulated (its children are openened, or
> annotation is attempted).
Hmm.. I'll think about it more how to do it.
>
> Here there is a second annoyance: on the detailed screen the 'annotate'
> entry is simply missing when the symbol has not been resolved. If I hit
> 'a' on the symbol entry itself in the graph view, then sometimes
> annotation works - sometimes it does not and there's no UI feedback
> whatsoever why it's not working.
>
> I'm not suggesting to change the keyboard flow - that is very smooth - but
> display information about failed annotations in the status line at the
> bottom of the screen would be very useful. Remember: it's _always_ an UI
> bug if the user hits a key and absolutely nothing happens. At minimum a
> low-key, non-intrusive 'key X not bound' message should appear in the
> status line bottom. Deterministic action/reaction sequences are utterly
> important when interacting with computers.
>
> Anyway, very nice progress with the tree here!
Hehe, thanks!
Namhyung
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)
2013-09-26 8:58 [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Namhyung Kim
` (8 preceding siblings ...)
2013-09-26 9:34 ` [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Ingo Molnar
@ 2013-09-26 13:46 ` David Ahern
2013-09-26 14:07 ` Arnaldo Carvalho de Melo
9 siblings, 1 reply; 24+ messages in thread
From: David Ahern @ 2013-09-26 13:46 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Linus Torvalds,
Frederic Weisbecker, Jiri Olsa
On 9/26/13 2:58 AM, Namhyung Kim wrote:
> Hello,
>
> This is a new version of callchain improvement patchset. I found and
> fixed bugs in the previous version. I verified that it produced
> exactly same output before and after applying rbtree conversion patch
> (#1). However after Frederic's new comm infrastructure patches are
> applied it'd be little different.
>
> The patches are on 'perf/callchain-v4' branch in my tree
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
Given recent breakage, has the patchset been run through any tests on
older kernels that do not support sample_id_all? e.g., 2.6.34 (WRL4).
What about tests with the other perf commands -- script to dump events,
trace on a file with with multiple processes -- to verify no impact on
comm output, especially multithreaded processes with named threads. I
can certainly do those tests in time, but can't guarantee a timeframe
and want to make sure it gets done before merging.
Thanks,
David
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)
2013-09-26 13:46 ` David Ahern
@ 2013-09-26 14:07 ` Arnaldo Carvalho de Melo
2013-09-27 2:10 ` Namhyung Kim
0 siblings, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-09-26 14:07 UTC (permalink / raw)
To: David Ahern
Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Namhyung Kim, LKML, Linus Torvalds, Frederic Weisbecker,
Jiri Olsa
Em Thu, Sep 26, 2013 at 07:46:36AM -0600, David Ahern escreveu:
> On 9/26/13 2:58 AM, Namhyung Kim wrote:
> >This is a new version of callchain improvement patchset. I found and
> >fixed bugs in the previous version. I verified that it produced
> >exactly same output before and after applying rbtree conversion patch
> >(#1). However after Frederic's new comm infrastructure patches are
> >applied it'd be little different.
> >The patches are on 'perf/callchain-v4' branch in my tree
> > git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> Given recent breakage, has the patchset been run through any tests
> on older kernels that do not support sample_id_all? e.g., 2.6.34
> (WRL4). What about tests with the other perf commands -- script to
> dump events, trace on a file with with multiple processes -- to
> verify no impact on comm output, especially multithreaded processes
> with named threads. I can certainly do those tests in time, but
> can't guarantee a timeframe and want to make sure it gets done
> before merging.
Right, and I don't think this is perf/urgent material at this point in
time, when merged will be for 3.13.
- Arnaldo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)
2013-09-26 14:07 ` Arnaldo Carvalho de Melo
@ 2013-09-27 2:10 ` Namhyung Kim
0 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2013-09-27 2:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Namhyung Kim, LKML, Linus Torvalds, Frederic Weisbecker,
Jiri Olsa
Hi,
On Thu, 26 Sep 2013 11:07:49 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Sep 26, 2013 at 07:46:36AM -0600, David Ahern escreveu:
>> On 9/26/13 2:58 AM, Namhyung Kim wrote:
>> >This is a new version of callchain improvement patchset. I found and
>> >fixed bugs in the previous version. I verified that it produced
>> >exactly same output before and after applying rbtree conversion patch
>> >(#1). However after Frederic's new comm infrastructure patches are
>> >applied it'd be little different.
>
>> >The patches are on 'perf/callchain-v4' branch in my tree
>
>> > git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
>> Given recent breakage, has the patchset been run through any tests
>> on older kernels that do not support sample_id_all? e.g., 2.6.34
>> (WRL4). What about tests with the other perf commands -- script to
>> dump events, trace on a file with with multiple processes -- to
>> verify no impact on comm output, especially multithreaded processes
>> with named threads. I can certainly do those tests in time, but
>> can't guarantee a timeframe and want to make sure it gets done
>> before merging.
>
> Right, and I don't think this is perf/urgent material at this point in
> time, when merged will be for 3.13.
Agreed. It definitely needs more testing. I'll try to do some testing
mentioned above.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 24+ messages in thread