Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1635289 > unrolled thread
| Started by | Milian Wolff <milian.wolff@kdab.com> |
|---|---|
| First post | 2017-05-03 23:40 +0200 |
| Last post | 2017-05-10 08:00 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v2] perf report: distinguish between inliners in the same function Milian Wolff <milian.wolff@kdab.com> - 2017-05-03 23:40 +0200
Re: [PATCH v2] perf report: distinguish between inliners in the same function Milian Wolff <milian.wolff@kdab.com> - 2017-05-08 10:50 +0200
Re: [PATCH v2] perf report: distinguish between inliners in the same function Arnaldo Carvalho de Melo <acme@redhat.com> - 2017-05-08 18:20 +0200
Re: [PATCH v2] perf report: distinguish between inliners in the same function Namhyung Kim <namhyung@kernel.org> - 2017-05-10 08:00 +0200
| From | Milian Wolff <milian.wolff@kdab.com> |
|---|---|
| Date | 2017-05-03 23:40 +0200 |
| Subject | [PATCH v2] perf report: distinguish between inliners in the same function |
| Message-ID | <tDaa6-2vi-17@gated-at.bofh.it> |
When different functions get inlined into the same function, we
want to show them individually in the reports. But when we group by
function, we would aggregate all IPs and would only keep the first
one in that function. E.g. for C++ code like the following:
~~~~~
#include <cmath>
#include <random>
#include <iostream>
using namespace std;
int main()
{
uniform_real_distribution<double> uniform(-1E5, 1E5);
default_random_engine engine;
double s = 0;
for (int i = 0; i < 10000000; ++i) {
s += uniform(engine);
}
cout << "random sum: " << s << '\n';
return 0;
}
~~~~~
Building it with `g++ -O2 -g` and recording some samples with
`perf record --call-graph dwarf` yields for me:
~~~~~
$ perf report --stdio --inline
# Overhead Command Shared Object Symbol
# ........ ......... ................. ................................
#
99.40% 99.11% a.out a.out [.] main
|
--99.11%--_start
__libc_start_main
main
...
~~~~~
Note how no inlined frames are actually shown, because the first
sample in main points to an IP that does not correspond to any
inlined frames.
With this patch applied, we instead get the following, much more
meaningful, reports.
~~~~~
$ perf report --stdio --inline --no-children
# Overhead Command Shared Object Symbol
# ........ ......... ................. ................................
#
99.11% a.out a.out [.] main
|
|--48.15%--main
| std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
| std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| main (inline)
| __libc_start_main
| _start
|
|--47.61%--main
| std::__detail::__mod<unsigned long, 2147483647ul, 16807ul, 0ul> (inline)
| std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>::operator() (inline)
| std::generate_canonical<double, 53ul, std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
| std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| main (inline)
| __libc_start_main
| _start
|
--3.35%--main
std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
main (inline)
__libc_start_main
_start
...
$ perf report --stdio --inline
# Children Self Command Shared Object Symbol
# ........ ........ ....... ................... ........................................................
#
99.40% 99.11% a.out a.out [.] main
|
--99.11%--_start
__libc_start_main
|
|--70.51%--main
| main (inline)
| std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
|
|--25.25%--main
| main (inline)
| std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
| std::generate_canonical<double, 53ul, std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
| std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>::operator() (inline)
| std::__detail::__mod<unsigned long, 2147483647ul, 16807ul, 0ul> (inline)
|
--3.35%--main
main (inline)
std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
~~~~~
Note that the latter top-down call graphs require us to traverse the
full inliner call chain when comparing for equality. Without it,
we would potentially combine the three distinct branches again, since
they share an equal first inline frame "main (inline)".
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Yao Jin <yao.jin@linux.intel.com>
Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
---
v2:
- compare the full stack of inlined frames to distinguish different branches
in top-down call-stack views which start with the same inlined frame(s)
tools/perf/util/callchain.c | 87 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 75 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 81fc29ac798f..9ab68682c6d0 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -618,15 +618,9 @@ enum match_result {
MATCH_GT,
};
-static enum match_result match_chain_srcline(struct callchain_cursor_node *node,
- struct callchain_list *cnode)
+static enum match_result match_chain_strings(const char *left,
+ const char *right)
{
- char *left = get_srcline(cnode->ms.map->dso,
- map__rip_2objdump(cnode->ms.map, cnode->ip),
- cnode->ms.sym, true, false);
- char *right = get_srcline(node->map->dso,
- map__rip_2objdump(node->map, node->ip),
- node->sym, true, false);
enum match_result ret = MATCH_EQ;
int cmp;
@@ -636,19 +630,82 @@ static enum match_result match_chain_srcline(struct callchain_cursor_node *node,
cmp = 1;
else if (left && !right)
cmp = -1;
- else if (cnode->ip == node->ip)
- cmp = 0;
else
- cmp = (cnode->ip < node->ip) ? -1 : 1;
+ return MATCH_ERROR;
if (cmp != 0)
ret = cmp < 0 ? MATCH_LT : MATCH_GT;
+ return ret;
+}
+
+static enum match_result match_chain_srcline(struct callchain_cursor_node *node,
+ struct callchain_list *cnode)
+{
+ char *left = get_srcline(cnode->ms.map->dso,
+ map__rip_2objdump(cnode->ms.map, cnode->ip),
+ cnode->ms.sym, true, false);
+ char *right = get_srcline(node->map->dso,
+ map__rip_2objdump(node->map, node->ip),
+ node->sym, true, false);
+ enum match_result ret = match_chain_strings(left, right);
+
free_srcline(left);
free_srcline(right);
return ret;
}
+static enum match_result match_chain_inliner(struct callchain_cursor_node *node,
+ struct callchain_list *cnode)
+{
+ u64 left_ip = map__rip_2objdump(cnode->ms.map, cnode->ip);
+ u64 right_ip = map__rip_2objdump(node->map, node->ip);
+ struct inline_node *left_node = NULL;
+ struct inline_node *right_node = NULL;
+ struct inline_list *left_entry = NULL;
+ struct inline_list *right_entry = NULL;
+ struct inline_list *left_last_entry = NULL;
+ struct inline_list *right_last_entry = NULL;
+ enum match_result ret = MATCH_EQ;
+
+ left_node = dso__parse_addr_inlines(cnode->ms.map->dso, left_ip);
+ if (!left_node)
+ return MATCH_ERROR;
+
+ right_node = dso__parse_addr_inlines(node->map->dso, right_ip);
+ if (!right_node) {
+ inline_node__delete(left_node);
+ return MATCH_ERROR;
+ }
+
+ left_entry = list_first_entry(&left_node->val,
+ struct inline_list, list);
+ left_last_entry = list_last_entry(&left_node->val,
+ struct inline_list, list);
+ right_entry = list_first_entry(&right_node->val,
+ struct inline_list, list);
+ right_last_entry = list_last_entry(&right_node->val,
+ struct inline_list, list);
+ while (ret == MATCH_EQ && (left_entry || right_entry)) {
+ ret = match_chain_strings(left_entry ? left_entry->funcname : NULL,
+ right_entry ? right_entry->funcname : NULL);
+
+ if (left_entry && left_entry != left_last_entry)
+ left_entry = list_next_entry(left_entry, list);
+ else
+ left_entry = NULL;
+
+ if (right_entry && right_entry != right_last_entry)
+ right_entry = list_next_entry(right_entry, list);
+ else
+ right_entry = NULL;
+ }
+
+ inline_node__delete(left_node);
+ inline_node__delete(right_node);
+ return ret;
+}
+
static enum match_result match_chain(struct callchain_cursor_node *node,
struct callchain_list *cnode)
{
@@ -671,7 +728,13 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
}
if (left == right) {
- if (node->branch) {
+ if (symbol_conf.inline_name && cnode->ip != node->ip) {
+ enum match_result match = match_chain_inliner(node,
+ cnode);
+
+ if (match != MATCH_ERROR)
+ return match;
+ } else if (node->branch) {
cnode->branch_count++;
if (node->branch_flags.predicted)
--
2.12.2
[toc] | [next] | [standalone]
| From | Milian Wolff <milian.wolff@kdab.com> |
|---|---|
| Date | 2017-05-08 10:50 +0200 |
| Message-ID | <tEMwF-1z9-7@gated-at.bofh.it> |
| In reply to | #1635289 |
[Multipart message — attachments visible in raw view] — view raw
On Mittwoch, 3. Mai 2017 23:35:36 CEST Milian Wolff wrote:
> When different functions get inlined into the same function, we
> want to show them individually in the reports. But when we group by
> function, we would aggregate all IPs and would only keep the first
> one in that function. E.g. for C++ code like the following:
>
> ~~~~~
> #include <cmath>
> #include <random>
> #include <iostream>
>
> using namespace std;
>
> int main()
> {
> uniform_real_distribution<double> uniform(-1E5, 1E5);
> default_random_engine engine;
> double s = 0;
> for (int i = 0; i < 10000000; ++i) {
> s += uniform(engine);
> }
> cout << "random sum: " << s << '\n';
> return 0;
> }
> ~~~~~
>
> Building it with `g++ -O2 -g` and recording some samples with
> `perf record --call-graph dwarf` yields for me:
>
> ~~~~~
> $ perf report --stdio --inline
> # Overhead Command Shared Object Symbol
> # ........ ......... ................. ................................
> #
> 99.40% 99.11% a.out a.out [.] main
>
> --99.11%--_start
> __libc_start_main
> main
> ...
> ~~~~~
>
> Note how no inlined frames are actually shown, because the first
> sample in main points to an IP that does not correspond to any
> inlined frames.
>
> With this patch applied, we instead get the following, much more
> meaningful, reports.
>
> ~~~~~
> $ perf report --stdio --inline --no-children
> # Overhead Command Shared Object Symbol
> # ........ ......... ................. ................................
> #
> 99.11% a.out a.out [.] main
>
> |--48.15%--main
> |
> | std::__detail::_Adaptor<std::linear_congruential_engi
> | ne<unsigned long, 16807ul, 0ul, 2147483647ul>,
> | double>::operator() (inline)
> | std::uniform_real_distribution<double>::operator()<s
> | td::linear_congruential_engine<unsigned long,
> | 16807ul, 0ul, 2147483647ul> > (inline)
> | std::uniform_real_distribution<double>::operator()<s
> | td::linear_congruential_engine<unsigned long,
> | 16807ul, 0ul, 2147483647ul> > (inline) main (inline)
> | __libc_start_main
> | _start
> |
> |--47.61%--main
> |
> | std::__detail::__mod<unsigned long, 2147483647ul,
> | 16807ul, 0ul> (inline)
> | std::linear_congruential_engine<unsigned long,
> | 16807ul, 0ul, 2147483647ul>::operator() (inline)
> | std::generate_canonical<double, 53ul,
> | std::linear_congruential_engine<unsigned long,
> | 16807ul, 0ul, 2147483647ul> > (inline)
> | std::__detail::_Adaptor<std::linear_congruential_eng
> | ine<unsigned long, 16807ul, 0ul, 2147483647ul>,
> | double>::operator() (inline)
> | std::uniform_real_distribution<double>::operator()<s
> | td::linear_congruential_engine<unsigned long,
> | 16807ul, 0ul, 2147483647ul> > (inline)
> | std::uniform_real_distribution<double>::operator()<s
> | td::linear_congruential_engine<unsigned long,
> | 16807ul, 0ul, 2147483647ul> > (inline) main (inline)
> | __libc_start_main
> | _start
>
> --3.35%--main
>
> std::uniform_real_distribution<double>::operator()<std::linear_congruential
> _engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline) main (inline)
> __libc_start_main
> _start
> ...
>
> $ perf report --stdio --inline
> # Children Self Command Shared Object Symbol
> # ........ ........ ....... ...................
> ........................................................ #
> 99.40% 99.11% a.out a.out [.] main
>
> --99.11%--_start
> __libc_start_main
>
> |--70.51%--main
> |
> | main (inline)
> | std::uniform_real_distribution<double>::op
> | erator()<std::linear_congruential_engine<u
> | nsigned long, 16807ul, 0ul, 2147483647ul>
> | > (inline)
> | std::uniform_real_distribution<double>::o
> | perator()<std::linear_congruential_engine<
> | unsigned long, 16807ul, 0ul, 2147483647ul>
> | > (inline)
> | std::__detail::_Adaptor<std::linear_congr
> | uential_engine<unsigned long, 16807ul,
> | 0ul, 2147483647ul>, double>::operator()
> | (inline) |
> |--25.25%--main
> |
> | main (inline)
> | std::uniform_real_distribution<double>::op
> | erator()<std::linear_congruential_engine<u
> | nsigned long, 16807ul, 0ul, 2147483647ul>
> | > (inline)
> | std::uniform_real_distribution<double>::o
> | perator()<std::linear_congruential_engine<
> | unsigned long, 16807ul, 0ul, 2147483647ul>
> | > (inline)
> | std::__detail::_Adaptor<std::linear_congr
> | uential_engine<unsigned long, 16807ul,
> | 0ul, 2147483647ul>, double>::operator()
> | (inline) std::generate_canonical<double,
> | 53ul,
> | std::linear_congruential_engine<unsigned
> | long, 16807ul, 0ul, 2147483647ul> >
> | (inline)
> | std::linear_congruential_engine<unsigned
> | long, 16807ul, 0ul,
> | 2147483647ul>::operator() (inline)
> | std::__detail::__mod<unsigned long,
> | 2147483647ul, 16807ul, 0ul> (inline)
> --3.35%--main
> main (inline)
>
> std::uniform_real_distribution<double>::operator()<std::linear_congruential
> _engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline) ~~~~~
>
> Note that the latter top-down call graphs require us to traverse the
> full inliner call chain when comparing for equality. Without it,
> we would potentially combine the three distinct branches again, since
> they share an equal first inline frame "main (inline)".
Ping? Any chance that I could get a review on this one please? It works really
well for me and greatly improves perf's usability for C++ code bases.
Thanks
--
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@redhat.com> |
|---|---|
| Date | 2017-05-08 18:20 +0200 |
| Subject | Re: [PATCH v2] perf report: distinguish between inliners in the same function |
| Message-ID | <tETya-6cU-15@gated-at.bofh.it> |
| In reply to | #1637310 |
Em Mon, May 08, 2017 at 10:45:18AM +0200, Milian Wolff escreveu: > On Mittwoch, 3. Mai 2017 23:35:36 CEST Milian Wolff wrote: > > When different functions get inlined into the same function, we > > want to show them individually in the reports. But when we group by > > function, we would aggregate all IPs and would only keep the first > > one in that function. E.g. for C++ code like the following: <SNIP> > Ping? Any chance that I could get a review on this one please? It works really > well for me and greatly improves perf's usability for C++ code bases. yeah, indeed that would be great to have someone reviewing this, - Arnaldo
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2017-05-10 08:00 +0200 |
| Subject | Re: [PATCH v2] perf report: distinguish between inliners in the same function |
| Message-ID | <tFsPg-5e7-11@gated-at.bofh.it> |
| In reply to | #1635289 |
Hi,
On Wed, May 03, 2017 at 11:35:36PM +0200, Milian Wolff wrote:
> When different functions get inlined into the same function, we
> want to show them individually in the reports. But when we group by
> function, we would aggregate all IPs and would only keep the first
> one in that function. E.g. for C++ code like the following:
>
> ~~~~~
> #include <cmath>
> #include <random>
> #include <iostream>
>
> using namespace std;
>
> int main()
> {
> uniform_real_distribution<double> uniform(-1E5, 1E5);
> default_random_engine engine;
> double s = 0;
> for (int i = 0; i < 10000000; ++i) {
> s += uniform(engine);
> }
> cout << "random sum: " << s << '\n';
> return 0;
> }
> ~~~~~
>
> Building it with `g++ -O2 -g` and recording some samples with
> `perf record --call-graph dwarf` yields for me:
>
> ~~~~~
> $ perf report --stdio --inline
> # Overhead Command Shared Object Symbol
> # ........ ......... ................. ................................
> #
> 99.40% 99.11% a.out a.out [.] main
> |
> --99.11%--_start
> __libc_start_main
> main
> ...
> ~~~~~
>
> Note how no inlined frames are actually shown, because the first
> sample in main points to an IP that does not correspond to any
> inlined frames.
>
> With this patch applied, we instead get the following, much more
> meaningful, reports.
>
> ~~~~~
> $ perf report --stdio --inline --no-children
> # Overhead Command Shared Object Symbol
> # ........ ......... ................. ................................
> #
> 99.11% a.out a.out [.] main
> |
> |--48.15%--main
> | std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
> | std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | main (inline)
> | __libc_start_main
> | _start
> |
> |--47.61%--main
> | std::__detail::__mod<unsigned long, 2147483647ul, 16807ul, 0ul> (inline)
> | std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>::operator() (inline)
> | std::generate_canonical<double, 53ul, std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
> | std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | main (inline)
> | __libc_start_main
> | _start
> |
> --3.35%--main
> std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> main (inline)
> __libc_start_main
> _start
> ...
>
> $ perf report --stdio --inline
> # Children Self Command Shared Object Symbol
> # ........ ........ ....... ................... ........................................................
> #
> 99.40% 99.11% a.out a.out [.] main
> |
> --99.11%--_start
> __libc_start_main
> |
> |--70.51%--main
> | main (inline)
> | std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
> |
> |--25.25%--main
> | main (inline)
> | std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
> | std::generate_canonical<double, 53ul, std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> | std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul>::operator() (inline)
> | std::__detail::__mod<unsigned long, 2147483647ul, 16807ul, 0ul> (inline)
> |
> --3.35%--main
> main (inline)
> std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned long, 16807ul, 0ul, 2147483647ul> > (inline)
> ~~~~~
>
> Note that the latter top-down call graphs require us to traverse the
> full inliner call chain when comparing for equality. Without it,
> we would potentially combine the three distinct branches again, since
> they share an equal first inline frame "main (inline)".
>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Yao Jin <yao.jin@linux.intel.com>
> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> ---
> v2:
> - compare the full stack of inlined frames to distinguish different branches
> in top-down call-stack views which start with the same inlined frame(s)
>
> tools/perf/util/callchain.c | 87 ++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 75 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 81fc29ac798f..9ab68682c6d0 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -618,15 +618,9 @@ enum match_result {
> MATCH_GT,
> };
>
> -static enum match_result match_chain_srcline(struct callchain_cursor_node *node,
> - struct callchain_list *cnode)
> +static enum match_result match_chain_strings(const char *left,
> + const char *right)
> {
> - char *left = get_srcline(cnode->ms.map->dso,
> - map__rip_2objdump(cnode->ms.map, cnode->ip),
> - cnode->ms.sym, true, false);
> - char *right = get_srcline(node->map->dso,
> - map__rip_2objdump(node->map, node->ip),
> - node->sym, true, false);
> enum match_result ret = MATCH_EQ;
> int cmp;
>
> @@ -636,19 +630,82 @@ static enum match_result match_chain_srcline(struct callchain_cursor_node *node,
> cmp = 1;
> else if (left && !right)
> cmp = -1;
> - else if (cnode->ip == node->ip)
> - cmp = 0;
> else
> - cmp = (cnode->ip < node->ip) ? -1 : 1;
> + return MATCH_ERROR;
>
> if (cmp != 0)
> ret = cmp < 0 ? MATCH_LT : MATCH_GT;
>
> + return ret;
> +}
> +
> +static enum match_result match_chain_srcline(struct callchain_cursor_node *node,
> + struct callchain_list *cnode)
> +{
> + char *left = get_srcline(cnode->ms.map->dso,
> + map__rip_2objdump(cnode->ms.map, cnode->ip),
> + cnode->ms.sym, true, false);
> + char *right = get_srcline(node->map->dso,
> + map__rip_2objdump(node->map, node->ip),
> + node->sym, true, false);
> + enum match_result ret = match_chain_strings(left, right);
I think we need to check inlined srcline as well. There might be a
case that two samples have different addresses (and from different
callchains) but happens to be mapped to a same srcline IMHO.
> +
> free_srcline(left);
> free_srcline(right);
> return ret;
> }
>
> +static enum match_result match_chain_inliner(struct callchain_cursor_node *node,
> + struct callchain_list *cnode)
> +{
> + u64 left_ip = map__rip_2objdump(cnode->ms.map, cnode->ip);
> + u64 right_ip = map__rip_2objdump(node->map, node->ip);
> + struct inline_node *left_node = NULL;
> + struct inline_node *right_node = NULL;
> + struct inline_list *left_entry = NULL;
> + struct inline_list *right_entry = NULL;
> + struct inline_list *left_last_entry = NULL;
> + struct inline_list *right_last_entry = NULL;
> + enum match_result ret = MATCH_EQ;
> +
> + left_node = dso__parse_addr_inlines(cnode->ms.map->dso, left_ip);
> + if (!left_node)
> + return MATCH_ERROR;
> +
> + right_node = dso__parse_addr_inlines(node->map->dso, right_ip);
> + if (!right_node) {
> + inline_node__delete(left_node);
> + return MATCH_ERROR;
> + }
> +
> + left_entry = list_first_entry(&left_node->val,
> + struct inline_list, list);
> + left_last_entry = list_last_entry(&left_node->val,
> + struct inline_list, list);
> + right_entry = list_first_entry(&right_node->val,
> + struct inline_list, list);
> + right_last_entry = list_last_entry(&right_node->val,
> + struct inline_list, list);
What about keeping number of entries in a inline_node so that we can
check the numbers for faster comparison?
> + while (ret == MATCH_EQ && (left_entry || right_entry)) {
> + ret = match_chain_strings(left_entry ? left_entry->funcname : NULL,
> + right_entry ? right_entry->funcname : NULL);
> +
> + if (left_entry && left_entry != left_last_entry)
> + left_entry = list_next_entry(left_entry, list);
> + else
> + left_entry = NULL;
> +
> + if (right_entry && right_entry != right_last_entry)
> + right_entry = list_next_entry(right_entry, list);
> + else
> + right_entry = NULL;
> + }
> +
> + inline_node__delete(left_node);
> + inline_node__delete(right_node);
> + return ret;
> +}
> +
> static enum match_result match_chain(struct callchain_cursor_node *node,
> struct callchain_list *cnode)
> {
> @@ -671,7 +728,13 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
> }
>
> if (left == right) {
> - if (node->branch) {
> + if (symbol_conf.inline_name && cnode->ip != node->ip) {
> + enum match_result match = match_chain_inliner(node,
> + cnode);
> +
> + if (match != MATCH_ERROR)
> + return match;
I guess it'd be better just returning the match result. Otherwise
MATCH_ERROR will be converted to MATCH_EQ..
Thanks,
Namhyung
> + } else if (node->branch) {
> cnode->branch_count++;
>
> if (node->branch_flags.predicted)
> --
> 2.12.2
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web