The options array in cmd_annotate() has duplicate --group options. It
only needs one and let's get rid of the other.
$ perf annotate -h 2>&1 | grep group
--group Show event group information together
--group Show event group information together
Fixes: 7ebaf4890f63 ("perf annotate: Support '--group' option")
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-annotate.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index f677671409b1..3e9f7e0596e8 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -810,8 +810,6 @@ int cmd_annotate(int argc, const char **argv)
"Enable symbol demangling"),
OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
"Enable kernel symbol demangling"),
- OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
- "Show event group information together"),
OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
"Show a column with the sum of periods"),
OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
--
2.44.0.396.g6e790dbe36-goog
For data type profiling output, it should be in sync with normal output
so make it display percentage for each field. Also use coloring scheme
for users to identify fields with big overhead easily.
Users can use --show-total-period or --show-nr-samples to change the
output style like in the normal perf annotate output.
Before:
$ perf annotate --data-type
Annotate type: 'struct task_struct' in [kernel.kallsyms] (34 samples):
============================================================================
samples offset size field
34 0 9792 struct task_struct {
2 0 24 struct thread_info thread_info {
0 0 8 long unsigned int flags;
1 8 8 long unsigned int syscall_work;
0 16 4 u32 status;
1 20 4 u32 cpu;
};
After:
$ perf annotate --data-type
Annotate type: 'struct task_struct' in [kernel.kallsyms] (34 samples):
============================================================================
Percent offset size field
100.00 0 9792 struct task_struct {
3.55 0 24 struct thread_info thread_info {
0.00 0 8 long unsigned int flags;
1.63 8 8 long unsigned int syscall_work;
0.00 16 4 u32 status;
1.91 20 4 u32 cpu;
};
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-annotate.c | 44 ++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 3e9f7e0596e8..16e1581207c9 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -42,6 +42,7 @@
#include <errno.h>
#include <linux/bitmap.h>
#include <linux/err.h>
+#include <inttypes.h>
struct perf_annotate {
struct perf_tool tool;
@@ -332,6 +333,8 @@ static void print_annotated_data_header(struct hist_entry *he, struct evsel *evs
struct dso *dso = map__dso(he->ms.map);
int nr_members = 1;
int nr_samples = he->stat.nr_events;
+ int width = 7;
+ const char *val_hdr = "Percent";
if (evsel__is_group_event(evsel)) {
struct hist_entry *pair;
@@ -353,8 +356,30 @@ static void print_annotated_data_header(struct hist_entry *he, struct evsel *evs
nr_members = evsel->core.nr_members;
}
+ if (symbol_conf.show_total_period) {
+ width = 11;
+ val_hdr = "Period";
+ } else if (symbol_conf.show_nr_samples) {
+ width = 7;
+ val_hdr = "Samples";
+ }
+
printf("============================================================================\n");
- printf("%*s %10s %10s %s\n", 11 * nr_members, "samples", "offset", "size", "field");
+ printf("%*s %10s %10s %s\n", (width + 1) * nr_members, val_hdr,
+ "offset", "size", "field");
+}
+
+static void print_annotated_data_value(struct type_hist *h, u64 period, int nr_samples)
+{
+ double percent = h->period ? (100.0 * period / h->period) : 0;
+ const char *color = get_percent_color(percent);
+
+ if (symbol_conf.show_total_period)
+ color_fprintf(stdout, color, " %11" PRIu64, period);
+ else if (symbol_conf.show_nr_samples)
+ color_fprintf(stdout, color, " %7d", nr_samples);
+ else
+ color_fprintf(stdout, color, " %7.2f", percent);
}
static void print_annotated_data_type(struct annotated_data_type *mem_type,
@@ -364,10 +389,14 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
struct annotated_member *child;
struct type_hist *h = mem_type->histograms[evsel->core.idx];
int i, nr_events = 1, samples = 0;
+ u64 period = 0;
+ int width = symbol_conf.show_total_period ? 11 : 7;
- for (i = 0; i < member->size; i++)
+ for (i = 0; i < member->size; i++) {
samples += h->addr[member->offset + i].nr_samples;
- printf(" %10d", samples);
+ period += h->addr[member->offset + i].period;
+ }
+ print_annotated_data_value(h, period, samples);
if (evsel__is_group_event(evsel)) {
struct evsel *pos;
@@ -376,9 +405,12 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
h = mem_type->histograms[pos->core.idx];
samples = 0;
- for (i = 0; i < member->size; i++)
+ period = 0;
+ for (i = 0; i < member->size; i++) {
samples += h->addr[member->offset + i].nr_samples;
- printf(" %10d", samples);
+ period += h->addr[member->offset + i].period;
+ }
+ print_annotated_data_value(h, period, samples);
}
nr_events = evsel->core.nr_members;
}
@@ -394,7 +426,7 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
print_annotated_data_type(mem_type, child, evsel, indent + 4);
if (!list_empty(&member->children))
- printf("%*s}", 11 * nr_events + 24 + indent, "");
+ printf("%*s}", (width + 1) * nr_events + 24 + indent, "");
printf(";\n");
}
--
2.44.0.396.g6e790dbe36-goog
On 2024-03-22 6:43 p.m., Namhyung Kim wrote:
> The options array in cmd_annotate() has duplicate --group options. It
> only needs one and let's get rid of the other.
>
> $ perf annotate -h 2>&1 | grep group
> --group Show event group information together
> --group Show event group information together
>
> Fixes: 7ebaf4890f63 ("perf annotate: Support '--group' option")
> Signed-off-by: Namhyung Kim <[email protected]>
For the series,
Reviewed-by: Kan Liang <[email protected]>
Thanks,
Kan
> ---
> tools/perf/builtin-annotate.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index f677671409b1..3e9f7e0596e8 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -810,8 +810,6 @@ int cmd_annotate(int argc, const char **argv)
> "Enable symbol demangling"),
> OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
> "Enable kernel symbol demangling"),
> - OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
> - "Show event group information together"),
> OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
> "Show a column with the sum of periods"),
> OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
On Mon, Mar 25, 2024 at 09:56:50AM -0400, Liang, Kan wrote:
>
>
> On 2024-03-22 6:43 p.m., Namhyung Kim wrote:
> > The options array in cmd_annotate() has duplicate --group options. It
> > only needs one and let's get rid of the other.
> >
> > $ perf annotate -h 2>&1 | grep group
> > --group Show event group information together
> > --group Show event group information together
> >
> > Fixes: 7ebaf4890f63 ("perf annotate: Support '--group' option")
> > Signed-off-by: Namhyung Kim <[email protected]>
>
> For the series,
>
> Reviewed-by: Kan Liang <[email protected]>
For 'b4' sake, next time please send the Reviewed-by to the cover letter
in the series so that it picks your Reviewed-by for all patches, not
just for the one that you replied to, as in this case.
I'm adding it to this whole series,
Thanks!
- Arnaldo