Re: [PATCH 18/27] perf tools: Add callback function to hist_entry_iter
On Thu, 29 May 2014 15:16:51 +0200, Jiri Olsa wrote:
> On Thu, May 29, 2014 at 12:58:21PM +0900, Namhyung Kim wrote:
>> +static int hist_iter__report_callback(struct hist_entry_iter *iter,
>> + struct addr_location *al, void *arg)
>> +{
>> +int err = 0;
>> +struct hist_entry *he = iter->he;
>> +struct perf_evsel *evsel = iter->evsel;
>> +struct report *rep = arg;
>> +
>> +if (ui__has_annotation())
>> +err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
>
> if we put the annotation stats in here, shouldn't we remove all other
> instancies of above call from:
>
> iter_finish_normal_entry
> iter_add_single_cumulative_entry
> iter_finish_mem_entry
Right, they were double counted.. :-/ I changed this as below and push
to perf/cumulate-v12. Would you please check it again?
Thanks,
Namhyung
>From 0be9139edf446a5d6d753698f42809b2212dcf76 Mon Sep 17 00:00:00 2001
From: Namhyung Kim
Date: Tue, 7 Jan 2014 17:02:25 +0900
Subject: [PATCH] perf tools: Add callback function to hist_entry_iter
The new ->add_entry_cb() will be called after an entry was added to
the histogram. It's used for code sharing between perf report and
perf top. Note that ops->add_*_entry() should set iter->he properly
in order to call the ->add_entry_cb.
Also pass @arg to the callback function. It'll be used by perf top
later.
Tested-by: Arun Sharma
Tested-by: Rodrigo Campos
Acked-by: Jiri Olsa
Cc: Frederic Weisbecker
Signed-off-by: Namhyung Kim
---
tools/perf/builtin-report.c | 61 -
tools/perf/tests/hists_filter.c | 2 +-
tools/perf/tests/hists_output.c | 2 +-
tools/perf/util/hist.c | 67 +++--
tools/perf/util/hist.h | 5 ++-
5 files changed, 84 insertions(+), 53 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 6cac509212ee..21d830bafff3 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -80,14 +80,59 @@ static int report__config(const char *var, const char
*value, void *cb)
return perf_default_config(var, value, cb);
}
-static void report__inc_stats(struct report *rep,
- struct hist_entry *he __maybe_unused)
+static void report__inc_stats(struct report *rep, struct hist_entry *he)
{
/*
-* We cannot access @he at this time. Just assume it's a new entry.
-* It'll be fixed once we have a callback mechanism in hist_iter.
+* The @he is either of a newly created one or an existing one
+* merging current sample. We only want to count a new one so
+* checking ->nr_events being 1.
*/
- rep->nr_entries++;
+ if (he->stat.nr_events == 1)
+ rep->nr_entries++;
+}
+
+static int hist_iter__report_callback(struct hist_entry_iter *iter,
+ struct addr_location *al, bool single,
+ void *arg)
+{
+ int err = 0;
+ struct report *rep = arg;
+ struct hist_entry *he = iter->he;
+ struct perf_evsel *evsel = iter->evsel;
+ struct mem_info *mi;
+ struct branch_info *bi;
+
+ report__inc_stats(rep, he);
+
+ if (!ui__has_annotation())
+ return 0;
+
+ if (sort__mode == SORT_MODE__BRANCH) {
+ bi = he->branch_info;
+ err = addr_map_symbol__inc_samples(>from, evsel->idx);
+ if (err)
+ goto out;
+
+ err = addr_map_symbol__inc_samples(>to, evsel->idx);
+
+ } else if (rep->mem_mode) {
+ mi = he->mem_info;
+ err = addr_map_symbol__inc_samples(>daddr, evsel->idx);
+ if (err)
+ goto out;
+
+ err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+
+ } else if (symbol_conf.cumulate_callchain) {
+ if (single)
+ err = hist_entry__inc_addr_samples(he, evsel->idx,
+ al->addr);
+ } else {
+ err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+ }
+
+out:
+ return err;
}
static int process_sample_event(struct perf_tool *tool,
@@ -100,6 +145,7 @@ static int process_sample_event(struct perf_tool *tool,
struct addr_location al;
struct hist_entry_iter iter = {
.hide_unresolved = rep->hide_unresolved,
+ .add_entry_cb = hist_iter__report_callback,
};
int ret;
@@ -127,9 +173,8 @@ static int process_sample_event(struct perf_tool *tool,
if (al.map != NULL)
al.map->dso->hit = 1;
- report__inc_stats(rep, NULL);
-
- ret = hist_entry_iter__add(, , evsel, sample, rep->max_stack);
+ ret = hist_entry_iter__add(, , evsel, sample, rep->max_stack,
+
Re: [PATCH 18/27] perf tools: Add callback function to hist_entry_iter
On Thu, May 29, 2014 at 12:58:21PM +0900, Namhyung Kim wrote:
> The new ->add_entry_cb() will be called after an entry was added to
> the histogram. It's used for code sharing between perf report and
> perf top. Note that ops->add_*_entry() should set iter->he properly
> in order to call the ->add_entry_cb.
>
> Also pass @arg to the callback function. It'll be used by perf top
> later.
>
> Tested-by: Arun Sharma
> Tested-by: Rodrigo Campos
> Acked-by: Jiri Olsa
> Cc: Frederic Weisbecker
> Signed-off-by: Namhyung Kim
> ---
> tools/perf/builtin-report.c | 33 +
> tools/perf/tests/hists_filter.c | 2 +-
> tools/perf/tests/hists_output.c | 2 +-
> tools/perf/util/hist.c | 19 +--
> tools/perf/util/hist.h | 5 -
> 5 files changed, 48 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 6cac509212ee..ed9f74dc2d27 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -80,14 +80,31 @@ static int report__config(const char *var, const char
> *value, void *cb)
> return perf_default_config(var, value, cb);
> }
>
> -static void report__inc_stats(struct report *rep,
> - struct hist_entry *he __maybe_unused)
> +static void report__inc_stats(struct report *rep, struct hist_entry *he)
> {
> /*
> - * We cannot access @he at this time. Just assume it's a new entry.
> - * It'll be fixed once we have a callback mechanism in hist_iter.
> + * The @he is either of a newly created one or an existing one
> + * merging current sample. We only want to count a new one so
> + * checking ->nr_events being 1.
>*/
> - rep->nr_entries++;
> + if (he->stat.nr_events == 1)
> + rep->nr_entries++;
> +}
> +
> +static int hist_iter__report_callback(struct hist_entry_iter *iter,
> + struct addr_location *al, void *arg)
> +{
> + int err = 0;
> + struct hist_entry *he = iter->he;
> + struct perf_evsel *evsel = iter->evsel;
> + struct report *rep = arg;
> +
> + if (ui__has_annotation())
> + err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
if we put the annotation stats in here, shouldn't we remove all other
instancies of above call from:
iter_finish_normal_entry
iter_add_single_cumulative_entry
iter_finish_mem_entry
SNIP
>
> @@ -883,10 +886,22 @@ int hist_entry_iter__add(struct hist_entry_iter *iter,
> struct addr_location *al,
> if (err)
> goto out;
>
> + if (iter->he && iter->add_entry_cb) {
> + err = iter->add_entry_cb(iter, al, arg);
> + if (err)
> + goto out;
> + }
> +
> while (iter->ops->next_entry(iter, al)) {
> err = iter->ops->add_next_entry(iter, al);
> if (err)
> break;
> +
> + if (iter->he && iter->add_entry_cb) {
> + err = iter->add_entry_cb(iter, al, arg);
> + if (err)
> + goto out;
> + }
hm, the callback code is identical.. do we want a function for this?
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 18/27] perf tools: Add callback function to hist_entry_iter
On Thu, May 29, 2014 at 12:58:21PM +0900, Namhyung Kim wrote:
The new -add_entry_cb() will be called after an entry was added to
the histogram. It's used for code sharing between perf report and
perf top. Note that ops-add_*_entry() should set iter-he properly
in order to call the -add_entry_cb.
Also pass @arg to the callback function. It'll be used by perf top
later.
Tested-by: Arun Sharma asha...@fb.com
Tested-by: Rodrigo Campos rodr...@sdfg.com.ar
Acked-by: Jiri Olsa jo...@redhat.com
Cc: Frederic Weisbecker fweis...@gmail.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
tools/perf/builtin-report.c | 33 +
tools/perf/tests/hists_filter.c | 2 +-
tools/perf/tests/hists_output.c | 2 +-
tools/perf/util/hist.c | 19 +--
tools/perf/util/hist.h | 5 -
5 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 6cac509212ee..ed9f74dc2d27 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -80,14 +80,31 @@ static int report__config(const char *var, const char
*value, void *cb)
return perf_default_config(var, value, cb);
}
-static void report__inc_stats(struct report *rep,
- struct hist_entry *he __maybe_unused)
+static void report__inc_stats(struct report *rep, struct hist_entry *he)
{
/*
- * We cannot access @he at this time. Just assume it's a new entry.
- * It'll be fixed once we have a callback mechanism in hist_iter.
+ * The @he is either of a newly created one or an existing one
+ * merging current sample. We only want to count a new one so
+ * checking -nr_events being 1.
*/
- rep-nr_entries++;
+ if (he-stat.nr_events == 1)
+ rep-nr_entries++;
+}
+
+static int hist_iter__report_callback(struct hist_entry_iter *iter,
+ struct addr_location *al, void *arg)
+{
+ int err = 0;
+ struct hist_entry *he = iter-he;
+ struct perf_evsel *evsel = iter-evsel;
+ struct report *rep = arg;
+
+ if (ui__has_annotation())
+ err = hist_entry__inc_addr_samples(he, evsel-idx, al-addr);
if we put the annotation stats in here, shouldn't we remove all other
instancies of above call from:
iter_finish_normal_entry
iter_add_single_cumulative_entry
iter_finish_mem_entry
SNIP
@@ -883,10 +886,22 @@ int hist_entry_iter__add(struct hist_entry_iter *iter,
struct addr_location *al,
if (err)
goto out;
+ if (iter-he iter-add_entry_cb) {
+ err = iter-add_entry_cb(iter, al, arg);
+ if (err)
+ goto out;
+ }
+
while (iter-ops-next_entry(iter, al)) {
err = iter-ops-add_next_entry(iter, al);
if (err)
break;
+
+ if (iter-he iter-add_entry_cb) {
+ err = iter-add_entry_cb(iter, al, arg);
+ if (err)
+ goto out;
+ }
hm, the callback code is identical.. do we want a function for this?
jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 18/27] perf tools: Add callback function to hist_entry_iter
On Thu, 29 May 2014 15:16:51 +0200, Jiri Olsa wrote:
On Thu, May 29, 2014 at 12:58:21PM +0900, Namhyung Kim wrote:
+static int hist_iter__report_callback(struct hist_entry_iter *iter,
+ struct addr_location *al, void *arg)
+{
+int err = 0;
+struct hist_entry *he = iter-he;
+struct perf_evsel *evsel = iter-evsel;
+struct report *rep = arg;
+
+if (ui__has_annotation())
+err = hist_entry__inc_addr_samples(he, evsel-idx, al-addr);
if we put the annotation stats in here, shouldn't we remove all other
instancies of above call from:
iter_finish_normal_entry
iter_add_single_cumulative_entry
iter_finish_mem_entry
Right, they were double counted.. :-/ I changed this as below and push
to perf/cumulate-v12. Would you please check it again?
Thanks,
Namhyung
From 0be9139edf446a5d6d753698f42809b2212dcf76 Mon Sep 17 00:00:00 2001
From: Namhyung Kim namhy...@kernel.org
Date: Tue, 7 Jan 2014 17:02:25 +0900
Subject: [PATCH] perf tools: Add callback function to hist_entry_iter
The new -add_entry_cb() will be called after an entry was added to
the histogram. It's used for code sharing between perf report and
perf top. Note that ops-add_*_entry() should set iter-he properly
in order to call the -add_entry_cb.
Also pass @arg to the callback function. It'll be used by perf top
later.
Tested-by: Arun Sharma asha...@fb.com
Tested-by: Rodrigo Campos rodr...@sdfg.com.ar
Acked-by: Jiri Olsa jo...@redhat.com
Cc: Frederic Weisbecker fweis...@gmail.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
tools/perf/builtin-report.c | 61 -
tools/perf/tests/hists_filter.c | 2 +-
tools/perf/tests/hists_output.c | 2 +-
tools/perf/util/hist.c | 67 +++--
tools/perf/util/hist.h | 5 ++-
5 files changed, 84 insertions(+), 53 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 6cac509212ee..21d830bafff3 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -80,14 +80,59 @@ static int report__config(const char *var, const char
*value, void *cb)
return perf_default_config(var, value, cb);
}
-static void report__inc_stats(struct report *rep,
- struct hist_entry *he __maybe_unused)
+static void report__inc_stats(struct report *rep, struct hist_entry *he)
{
/*
-* We cannot access @he at this time. Just assume it's a new entry.
-* It'll be fixed once we have a callback mechanism in hist_iter.
+* The @he is either of a newly created one or an existing one
+* merging current sample. We only want to count a new one so
+* checking -nr_events being 1.
*/
- rep-nr_entries++;
+ if (he-stat.nr_events == 1)
+ rep-nr_entries++;
+}
+
+static int hist_iter__report_callback(struct hist_entry_iter *iter,
+ struct addr_location *al, bool single,
+ void *arg)
+{
+ int err = 0;
+ struct report *rep = arg;
+ struct hist_entry *he = iter-he;
+ struct perf_evsel *evsel = iter-evsel;
+ struct mem_info *mi;
+ struct branch_info *bi;
+
+ report__inc_stats(rep, he);
+
+ if (!ui__has_annotation())
+ return 0;
+
+ if (sort__mode == SORT_MODE__BRANCH) {
+ bi = he-branch_info;
+ err = addr_map_symbol__inc_samples(bi-from, evsel-idx);
+ if (err)
+ goto out;
+
+ err = addr_map_symbol__inc_samples(bi-to, evsel-idx);
+
+ } else if (rep-mem_mode) {
+ mi = he-mem_info;
+ err = addr_map_symbol__inc_samples(mi-daddr, evsel-idx);
+ if (err)
+ goto out;
+
+ err = hist_entry__inc_addr_samples(he, evsel-idx, al-addr);
+
+ } else if (symbol_conf.cumulate_callchain) {
+ if (single)
+ err = hist_entry__inc_addr_samples(he, evsel-idx,
+ al-addr);
+ } else {
+ err = hist_entry__inc_addr_samples(he, evsel-idx, al-addr);
+ }
+
+out:
+ return err;
}
static int process_sample_event(struct perf_tool *tool,
@@ -100,6 +145,7 @@ static int process_sample_event(struct perf_tool *tool,
struct addr_location al;
struct hist_entry_iter iter = {
.hide_unresolved = rep-hide_unresolved,
+ .add_entry_cb = hist_iter__report_callback,
};
int ret;
@@ -127,9 +173,8 @@ static int process_sample_event(struct perf_tool *tool,
if (al.map != NULL)
al.map-dso-hit = 1;
- report__inc_stats(rep, NULL);
-
- ret = hist_entry_iter__add(iter, al, evsel, sample, rep-max_stack);
+ ret =
