* Re: [PATCH] perf report: Allow sorting by symbol size.
2017-02-24 13:32 [PATCH] perf report: Allow sorting by symbol size charles.baylis
@ 2017-02-24 13:35 ` Arnaldo Carvalho de Melo
2017-02-24 19:22 ` Arnaldo Carvalho de Melo
2017-03-07 8:04 ` [tip:perf/core] perf tools: " tip-bot for Charles Baylis
2 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-02-24 13:35 UTC (permalink / raw)
To: charles.baylis
Cc: peterz, mingo, alexander.shishkin, linux-kernel, maxim.kuvyrkov
Em Fri, Feb 24, 2017 at 01:32:56PM +0000, charles.baylis@linaro.org escreveu:
> From: Charles Baylis <charles.baylis@linaro.org>
>
> Add new sort key 'symbol_size' to allow user to sort by
> symbol size, or (more usefully) display the symbol size
> using --fields=...,symbol_size.
Hey, really nice, will test and apply, and this opens the door for many
other keys, and to combine symbol_size with other keys :-)
- Arnaldo
> Signed-off-by: Charles Baylis <charles.baylis@linaro.org>
> ---
> tools/perf/Documentation/perf-report.txt | 1 +
> tools/perf/util/hist.h | 1 +
> tools/perf/util/sort.c | 46 ++++++++++++++++++++++++++++++++
> tools/perf/util/sort.h | 1 +
> 4 files changed, 49 insertions(+)
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index f2914f0..d2a8c15 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -76,6 +76,7 @@ OPTIONS
> - pid: command and tid of the task
> - dso: name of library or module executed at the time of sample
> - symbol: name of function executed at the time of sample
> + - symbol_size: size of function executed at the time of sample
> - parent: name of function matched to the parent regex filter. Unmatched
> entries are displayed as "[other]".
> - cpu: cpu number the task ran at the time of sample
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 28c216e..2e839bf 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -57,6 +57,7 @@ enum hist_column {
> HISTC_SRCLINE_FROM,
> HISTC_SRCLINE_TO,
> HISTC_TRACE,
> + HISTC_SYM_SIZE,
> HISTC_NR_COLS, /* Last entry */
> };
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index df622f4..0ad397c 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1396,6 +1396,51 @@ struct sort_entry sort_transaction = {
> .se_width_idx = HISTC_TRANSACTION,
> };
>
> +/* --sort symbol_size */
> +
> +static int64_t _sort__sym_size_cmp(struct symbol *sym_l, struct symbol *sym_r)
> +{
> + int64_t size_l = sym_l != NULL ? sym_l->end - sym_l->start : 0;
> + int64_t size_r = sym_r != NULL ? sym_r->end - sym_r->start : 0;
> +
> + return size_l < size_r ? -1 :
> + size_l == size_r ? 0 : 1;
> +}
> +
> +static int64_t
> +sort__sym_size_cmp(struct hist_entry *left, struct hist_entry *right)
> +{
> + return _sort__sym_size_cmp(right->ms.sym, left->ms.sym);
> +}
> +
> +static int _hist_entry__sym_size_snprintf(struct symbol *sym, char *bf,
> + size_t bf_size, unsigned int width)
> +{
> + if (sym) {
> + int64_t sym_size = sym->end - sym->start;
> +
> + return repsep_snprintf(bf, bf_size, "%*lld", width,
> + (long long)sym_size);
> + } else {
> + return repsep_snprintf(bf, bf_size, "%*s", width,
> + "unknown");
> + }
> +}
> +
> +static int hist_entry__sym_size_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width)
> +{
> + return _hist_entry__sym_size_snprintf(he->ms.sym, bf, size, width);
> +}
> +
> +struct sort_entry sort_sym_size = {
> + .se_header = "Symbol size",
> + .se_cmp = sort__sym_size_cmp,
> + .se_snprintf = hist_entry__sym_size_snprintf,
> + .se_width_idx = HISTC_SYM_SIZE,
> +};
> +
> +
> struct sort_dimension {
> const char *name;
> struct sort_entry *entry;
> @@ -1418,6 +1463,7 @@ static struct sort_dimension common_sort_dimensions[] = {
> DIM(SORT_GLOBAL_WEIGHT, "weight", sort_global_weight),
> DIM(SORT_TRANSACTION, "transaction", sort_transaction),
> DIM(SORT_TRACE, "trace", sort_trace),
> + DIM(SORT_SYM_SIZE, "symbol_size", sort_sym_size),
> };
>
> #undef DIM
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 7aff317..acb2c57 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -211,6 +211,7 @@ enum sort_type {
> SORT_GLOBAL_WEIGHT,
> SORT_TRANSACTION,
> SORT_TRACE,
> + SORT_SYM_SIZE,
>
> /* branch stack specific sort keys */
> __SORT_BRANCH_STACK,
> --
> 2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf report: Allow sorting by symbol size.
2017-02-24 13:32 [PATCH] perf report: Allow sorting by symbol size charles.baylis
2017-02-24 13:35 ` Arnaldo Carvalho de Melo
@ 2017-02-24 19:22 ` Arnaldo Carvalho de Melo
2017-02-25 13:06 ` Charles Baylis
2017-03-07 8:04 ` [tip:perf/core] perf tools: " tip-bot for Charles Baylis
2 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-02-24 19:22 UTC (permalink / raw)
To: charles.baylis
Cc: peterz, mingo, alexander.shishkin, linux-kernel, maxim.kuvyrkov
Em Fri, Feb 24, 2017 at 01:32:56PM +0000, charles.baylis@linaro.org escreveu:
> From: Charles Baylis <charles.baylis@linaro.org>
>
> Add new sort key 'symbol_size' to allow user to sort by
> symbol size, or (more usefully) display the symbol size
> using --fields=...,symbol_size.
>
> Signed-off-by: Charles Baylis <charles.baylis@linaro.org>
> ---
> tools/perf/Documentation/perf-report.txt | 1 +
> tools/perf/util/hist.h | 1 +
> tools/perf/util/sort.c | 46 ++++++++++++++++++++++++++++++++
> tools/perf/util/sort.h | 1 +
> 4 files changed, 49 insertions(+)
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index f2914f0..d2a8c15 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -76,6 +76,7 @@ OPTIONS
> - pid: command and tid of the task
> - dso: name of library or module executed at the time of sample
> - symbol: name of function executed at the time of sample
> + - symbol_size: size of function executed at the time of sample
> - parent: name of function matched to the parent regex filter. Unmatched
> entries are displayed as "[other]".
> - cpu: cpu number the task ran at the time of sample
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 28c216e..2e839bf 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -57,6 +57,7 @@ enum hist_column {
> HISTC_SRCLINE_FROM,
> HISTC_SRCLINE_TO,
> HISTC_TRACE,
> + HISTC_SYM_SIZE,
> HISTC_NR_COLS, /* Last entry */
> };
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index df622f4..0ad397c 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1396,6 +1396,51 @@ struct sort_entry sort_transaction = {
> .se_width_idx = HISTC_TRANSACTION,
> };
>
> +/* --sort symbol_size */
> +
> +static int64_t _sort__sym_size_cmp(struct symbol *sym_l, struct symbol *sym_r)
> +{
> + int64_t size_l = sym_l != NULL ? sym_l->end - sym_l->start : 0;
> + int64_t size_r = sym_r != NULL ? sym_r->end - sym_r->start : 0;
We have symbol__size(), no need to open code it, I'll fix it
> +
> + return size_l < size_r ? -1 :
> + size_l == size_r ? 0 : 1;
> +}
> +
> +static int64_t
> +sort__sym_size_cmp(struct hist_entry *left, struct hist_entry *right)
> +{
> + return _sort__sym_size_cmp(right->ms.sym, left->ms.sym);
> +}
> +
> +static int _hist_entry__sym_size_snprintf(struct symbol *sym, char *bf,
> + size_t bf_size, unsigned int width)
> +{
> + if (sym) {
> + int64_t sym_size = sym->end - sym->start;
Ditto
> +
> + return repsep_snprintf(bf, bf_size, "%*lld", width,
> + (long long)sym_size);
Humm, why use lld instead of plain d, that way you can get rid of that
(long long)? I'm fixing this as well
> + } else {
> + return repsep_snprintf(bf, bf_size, "%*s", width,
> + "unknown");
> + }
> +}
> +
> +static int hist_entry__sym_size_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width)
> +{
> + return _hist_entry__sym_size_snprintf(he->ms.sym, bf, size, width);
> +}
> +
> +struct sort_entry sort_sym_size = {
> + .se_header = "Symbol size",
> + .se_cmp = sort__sym_size_cmp,
> + .se_snprintf = hist_entry__sym_size_snprintf,
> + .se_width_idx = HISTC_SYM_SIZE,
> +};
> +
> +
> struct sort_dimension {
> const char *name;
> struct sort_entry *entry;
> @@ -1418,6 +1463,7 @@ static struct sort_dimension common_sort_dimensions[] = {
> DIM(SORT_GLOBAL_WEIGHT, "weight", sort_global_weight),
> DIM(SORT_TRANSACTION, "transaction", sort_transaction),
> DIM(SORT_TRACE, "trace", sort_trace),
> + DIM(SORT_SYM_SIZE, "symbol_size", sort_sym_size),
> };
>
> #undef DIM
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 7aff317..acb2c57 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -211,6 +211,7 @@ enum sort_type {
> SORT_GLOBAL_WEIGHT,
> SORT_TRANSACTION,
> SORT_TRACE,
> + SORT_SYM_SIZE,
>
> /* branch stack specific sort keys */
> __SORT_BRANCH_STACK,
> --
> 2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf report: Allow sorting by symbol size.
2017-02-24 19:22 ` Arnaldo Carvalho de Melo
@ 2017-02-25 13:06 ` Charles Baylis
0 siblings, 0 replies; 5+ messages in thread
From: Charles Baylis @ 2017-02-25 13:06 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: peterz, mingo, alexander.shishkin, linux-kernel, Maxim Kuvyrkov
On 24 February 2017 at 19:22, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>
>> +/* --sort symbol_size */
>> +
>> +static int64_t _sort__sym_size_cmp(struct symbol *sym_l, struct symbol *sym_r)
>> +{
>> + int64_t size_l = sym_l != NULL ? sym_l->end - sym_l->start : 0;
>> + int64_t size_r = sym_r != NULL ? sym_r->end - sym_r->start : 0;
>
> We have symbol__size(), no need to open code it, I'll fix it
OK thanks.
>> +
>> + return size_l < size_r ? -1 :
>> + size_l == size_r ? 0 : 1;
>> +}
>> +
>> +static int64_t
>> +sort__sym_size_cmp(struct hist_entry *left, struct hist_entry *right)
>> +{
>> + return _sort__sym_size_cmp(right->ms.sym, left->ms.sym);
>> +}
>> +
>> +static int _hist_entry__sym_size_snprintf(struct symbol *sym, char *bf,
>> + size_t bf_size, unsigned int width)
>> +{
>> + if (sym) {
>> + int64_t sym_size = sym->end - sym->start;
>
>
> Ditto
>
>> +
>> + return repsep_snprintf(bf, bf_size, "%*lld", width,
>> + (long long)sym_size);
>
> Humm, why use lld instead of plain d, that way you can get rid of that
> (long long)? I'm fixing this as well
struct symbol defines start and end as u64, so it seemed prudent to
treat the size as 64 bit too. However, since symbol__size() returns
size_t, the best thing to write is probably
return repsep_snprintf(bf, bf_size, "%*zu", width, symbol__size(sym));
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:perf/core] perf tools: Allow sorting by symbol size
2017-02-24 13:32 [PATCH] perf report: Allow sorting by symbol size charles.baylis
2017-02-24 13:35 ` Arnaldo Carvalho de Melo
2017-02-24 19:22 ` Arnaldo Carvalho de Melo
@ 2017-03-07 8:04 ` tip-bot for Charles Baylis
2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Charles Baylis @ 2017-03-07 8:04 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, acme, charles.baylis, maxim.kuvyrkov, mingo, peterz,
alexander.shishkin, linux-kernel, tglx
Commit-ID: 7768f8dada66d6052fccbc2ddc375f3e650455b9
Gitweb: http://git.kernel.org/tip/7768f8dada66d6052fccbc2ddc375f3e650455b9
Author: Charles Baylis <charles.baylis@linaro.org>
AuthorDate: Fri, 24 Feb 2017 13:32:56 +0000
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 3 Mar 2017 19:07:16 -0300
perf tools: Allow sorting by symbol size
Add new sort key 'symbol_size' to allow user to sort by symbol size, or
(more usefully) display the symbol size using --fields=...,symbol_size.
Committer note:
Testing it together with the recently added -q, to remove the headers,
and using the '+' sign with -s, to add the symbol_size sort order to
the default, which is '-s/--sort comm,dso,symbol':
# perf report -q -s +symbol_size | head -10
10.39% swapper [kernel.vmlinux] [k] intel_idle 270
3.45% swapper [kernel.vmlinux] [k] update_blocked_averages 1546
2.61% swapper [kernel.vmlinux] [k] update_load_avg 1292
2.36% swapper [kernel.vmlinux] [k] update_cfs_shares 240
1.83% swapper [kernel.vmlinux] [k] __hrtimer_run_queues 606
1.74% swapper [kernel.vmlinux] [k] update_cfs_rq_load_avg. 1187
1.66% swapper [kernel.vmlinux] [k] apic_timer_interrupt 152
1.60% CPU 0/KVM [kvm] [k] kvm_set_msr_common 3046
1.60% gnome-shell libglib-2.0.so.0 [.] g_slist_find 37
1.46% gnome-termina libglib-2.0.so.0 [.] g_hash_table_lookup 370
#
Signed-off-by: Charles Baylis <charles.baylis@linaro.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1487943176-13840-1-git-send-email-charles.baylis@linaro.org
[ Use symbol__size(), remove needless %lld + (long long) casting ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Documentation/perf-report.txt | 1 +
tools/perf/util/hist.h | 1 +
tools/perf/util/sort.c | 41 ++++++++++++++++++++++++++++++++
tools/perf/util/sort.h | 1 +
4 files changed, 44 insertions(+)
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index c04cc06..33f9190 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -80,6 +80,7 @@ OPTIONS
- pid: command and tid of the task
- dso: name of library or module executed at the time of sample
- symbol: name of function executed at the time of sample
+ - symbol_size: size of function executed at the time of sample
- parent: name of function matched to the parent regex filter. Unmatched
entries are displayed as "[other]".
- cpu: cpu number the task ran at the time of sample
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 28c216e..2e839bf 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -57,6 +57,7 @@ enum hist_column {
HISTC_SRCLINE_FROM,
HISTC_SRCLINE_TO,
HISTC_TRACE,
+ HISTC_SYM_SIZE,
HISTC_NR_COLS, /* Last entry */
};
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 0ff6222..f8f16c0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1396,6 +1396,46 @@ struct sort_entry sort_transaction = {
.se_width_idx = HISTC_TRANSACTION,
};
+/* --sort symbol_size */
+
+static int64_t _sort__sym_size_cmp(struct symbol *sym_l, struct symbol *sym_r)
+{
+ int64_t size_l = sym_l != NULL ? symbol__size(sym_l) : 0;
+ int64_t size_r = sym_r != NULL ? symbol__size(sym_r) : 0;
+
+ return size_l < size_r ? -1 :
+ size_l == size_r ? 0 : 1;
+}
+
+static int64_t
+sort__sym_size_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+ return _sort__sym_size_cmp(right->ms.sym, left->ms.sym);
+}
+
+static int _hist_entry__sym_size_snprintf(struct symbol *sym, char *bf,
+ size_t bf_size, unsigned int width)
+{
+ if (sym)
+ return repsep_snprintf(bf, bf_size, "%*d", width, symbol__size(sym));
+
+ return repsep_snprintf(bf, bf_size, "%*s", width, "unknown");
+}
+
+static int hist_entry__sym_size_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width)
+{
+ return _hist_entry__sym_size_snprintf(he->ms.sym, bf, size, width);
+}
+
+struct sort_entry sort_sym_size = {
+ .se_header = "Symbol size",
+ .se_cmp = sort__sym_size_cmp,
+ .se_snprintf = hist_entry__sym_size_snprintf,
+ .se_width_idx = HISTC_SYM_SIZE,
+};
+
+
struct sort_dimension {
const char *name;
struct sort_entry *entry;
@@ -1418,6 +1458,7 @@ static struct sort_dimension common_sort_dimensions[] = {
DIM(SORT_GLOBAL_WEIGHT, "weight", sort_global_weight),
DIM(SORT_TRANSACTION, "transaction", sort_transaction),
DIM(SORT_TRACE, "trace", sort_trace),
+ DIM(SORT_SYM_SIZE, "symbol_size", sort_sym_size),
};
#undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 796c847..f583325 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -211,6 +211,7 @@ enum sort_type {
SORT_GLOBAL_WEIGHT,
SORT_TRANSACTION,
SORT_TRACE,
+ SORT_SYM_SIZE,
/* branch stack specific sort keys */
__SORT_BRANCH_STACK,
^ permalink raw reply related [flat|nested] 5+ messages in thread