2024-02-28 00:52:53

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/4] perf annotate: Calculate instruction overhead using hashmap

Use annotated_source.samples hashmap instead of addr array in the
struct sym_hist.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/ui/gtk/annotate.c | 14 +++++++++---
tools/perf/util/annotate.c | 44 ++++++++++++++++++++++++------------
tools/perf/util/annotate.h | 11 +++++++++
3 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index 394861245fd3..93ce3d47e47e 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -28,21 +28,29 @@ static const char *const col_names[] = {
static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
struct disasm_line *dl, int evidx)
{
+ struct annotation *notes;
struct sym_hist *symhist;
+ struct sym_hist_entry *entry;
double percent = 0.0;
const char *markup;
int ret = 0;
+ u64 nr_samples = 0;

strcpy(buf, "");

if (dl->al.offset == (s64) -1)
return 0;

- symhist = annotation__histogram(symbol__annotation(sym), evidx);
- if (!symbol_conf.event_group && !symhist->addr[dl->al.offset].nr_samples)
+ notes = symbol__annotation(sym);
+ symhist = annotation__histogram(notes, evidx);
+ entry = annotated_source__hist_entry(notes->src, evidx, dl->al.offset);
+ if (entry)
+ nr_samples = entry->nr_samples;
+
+ if (!symbol_conf.event_group && nr_samples == 0)
return 0;

- percent = 100.0 * symhist->addr[dl->al.offset].nr_samples / symhist->nr_samples;
+ percent = 100.0 * nr_samples / symhist->nr_samples;

markup = perf_gtk__get_percent_color(percent);
if (markup)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 7a70e4d35c9b..e7859f756252 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2368,17 +2368,25 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
return err;
}

-static void calc_percent(struct sym_hist *sym_hist,
- struct hists *hists,
+static void calc_percent(struct annotation *notes,
+ struct evsel *evsel,
struct annotation_data *data,
s64 offset, s64 end)
{
+ struct hists *hists = evsel__hists(evsel);
+ int evidx = evsel->core.idx;
+ struct sym_hist *sym_hist = annotation__histogram(notes, evidx);
unsigned int hits = 0;
u64 period = 0;

while (offset < end) {
- hits += sym_hist->addr[offset].nr_samples;
- period += sym_hist->addr[offset].period;
+ struct sym_hist_entry *entry;
+
+ entry = annotated_source__hist_entry(notes->src, evidx, offset);
+ if (entry) {
+ hits += entry->nr_samples;
+ period += entry->period;
+ }
++offset;
}

@@ -2415,16 +2423,13 @@ static void annotation__calc_percent(struct annotation *notes,
end = next ? next->offset : len;

for_each_group_evsel(evsel, leader) {
- struct hists *hists = evsel__hists(evsel);
struct annotation_data *data;
- struct sym_hist *sym_hist;

BUG_ON(i >= al->data_nr);

- sym_hist = annotation__histogram(notes, evsel->core.idx);
data = &al->data[i++];

- calc_percent(sym_hist, hists, data, al->offset, end);
+ calc_percent(notes, evsel, data, al->offset, end);
}
}
}
@@ -2619,14 +2624,19 @@ static void print_summary(struct rb_root *root, const char *filename)

static void symbol__annotate_hits(struct symbol *sym, struct evsel *evsel)
{
+ int evidx = evsel->core.idx;
struct annotation *notes = symbol__annotation(sym);
- struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
+ struct sym_hist *h = annotation__histogram(notes, evidx);
u64 len = symbol__size(sym), offset;

- for (offset = 0; offset < len; ++offset)
- if (h->addr[offset].nr_samples != 0)
+ for (offset = 0; offset < len; ++offset) {
+ struct sym_hist_entry *entry;
+
+ entry = annotated_source__hist_entry(notes->src, evidx, offset);
+ if (entry && entry->nr_samples != 0)
printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2,
- sym->start + offset, h->addr[offset].nr_samples);
+ sym->start + offset, entry->nr_samples);
+ }
printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->nr_samples", h->nr_samples);
}

@@ -2855,8 +2865,14 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx)

h->nr_samples = 0;
for (offset = 0; offset < len; ++offset) {
- h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8;
- h->nr_samples += h->addr[offset].nr_samples;
+ struct sym_hist_entry *entry;
+
+ entry = annotated_source__hist_entry(notes->src, evidx, offset);
+ if (entry == NULL)
+ continue;
+
+ entry->nr_samples = entry->nr_samples * 7 / 8;
+ h->nr_samples += entry->nr_samples;
}
}

diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index a2b0c8210740..3362980a5d3d 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -356,6 +356,17 @@ static inline struct sym_hist *annotation__histogram(struct annotation *notes, i
return annotated_source__histogram(notes->src, idx);
}

+static inline struct sym_hist_entry *
+annotated_source__hist_entry(struct annotated_source *src, int idx, u64 offset)
+{
+ struct sym_hist_entry *entry;
+ long key = offset << 16 | idx;
+
+ if (!hashmap__find(src->samples, key, &entry))
+ return NULL;
+ return entry;
+}
+
static inline struct annotation *symbol__annotation(struct symbol *sym)
{
return (void *)sym - symbol_conf.priv_size;
--
2.44.0.rc1.240.g4c46232300-goog



2024-02-28 05:44:00

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 2/4] perf annotate: Calculate instruction overhead using hashmap

On Tue, Feb 27, 2024 at 5:27 PM Namhyung Kim <[email protected]> wrote:
>
> Use annotated_source.samples hashmap instead of addr array in the
> struct sym_hist.
>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/ui/gtk/annotate.c | 14 +++++++++---
> tools/perf/util/annotate.c | 44 ++++++++++++++++++++++++------------
> tools/perf/util/annotate.h | 11 +++++++++
> 3 files changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
> index 394861245fd3..93ce3d47e47e 100644
> --- a/tools/perf/ui/gtk/annotate.c
> +++ b/tools/perf/ui/gtk/annotate.c
> @@ -28,21 +28,29 @@ static const char *const col_names[] = {
> static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
> struct disasm_line *dl, int evidx)
> {
> + struct annotation *notes;
> struct sym_hist *symhist;
> + struct sym_hist_entry *entry;
> double percent = 0.0;
> const char *markup;
> int ret = 0;
> + u64 nr_samples = 0;
>
> strcpy(buf, "");
>
> if (dl->al.offset == (s64) -1)
> return 0;
>
> - symhist = annotation__histogram(symbol__annotation(sym), evidx);
> - if (!symbol_conf.event_group && !symhist->addr[dl->al.offset].nr_samples)
> + notes = symbol__annotation(sym);
> + symhist = annotation__histogram(notes, evidx);
> + entry = annotated_source__hist_entry(notes->src, evidx, dl->al.offset);
> + if (entry)
> + nr_samples = entry->nr_samples;
> +
> + if (!symbol_conf.event_group && nr_samples == 0)
> return 0;
>
> - percent = 100.0 * symhist->addr[dl->al.offset].nr_samples / symhist->nr_samples;
> + percent = 100.0 * nr_samples / symhist->nr_samples;
>
> markup = perf_gtk__get_percent_color(percent);
> if (markup)
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 7a70e4d35c9b..e7859f756252 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2368,17 +2368,25 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> return err;
> }
>
> -static void calc_percent(struct sym_hist *sym_hist,
> - struct hists *hists,
> +static void calc_percent(struct annotation *notes,
> + struct evsel *evsel,
> struct annotation_data *data,
> s64 offset, s64 end)
> {
> + struct hists *hists = evsel__hists(evsel);
> + int evidx = evsel->core.idx;
> + struct sym_hist *sym_hist = annotation__histogram(notes, evidx);
> unsigned int hits = 0;
> u64 period = 0;
>
> while (offset < end) {
> - hits += sym_hist->addr[offset].nr_samples;
> - period += sym_hist->addr[offset].period;
> + struct sym_hist_entry *entry;
> +
> + entry = annotated_source__hist_entry(notes->src, evidx, offset);
> + if (entry) {
> + hits += entry->nr_samples;
> + period += entry->period;
> + }
> ++offset;
> }
>
> @@ -2415,16 +2423,13 @@ static void annotation__calc_percent(struct annotation *notes,
> end = next ? next->offset : len;
>
> for_each_group_evsel(evsel, leader) {
> - struct hists *hists = evsel__hists(evsel);
> struct annotation_data *data;
> - struct sym_hist *sym_hist;
>
> BUG_ON(i >= al->data_nr);
>
> - sym_hist = annotation__histogram(notes, evsel->core.idx);
> data = &al->data[i++];
>
> - calc_percent(sym_hist, hists, data, al->offset, end);
> + calc_percent(notes, evsel, data, al->offset, end);
> }
> }
> }
> @@ -2619,14 +2624,19 @@ static void print_summary(struct rb_root *root, const char *filename)
>
> static void symbol__annotate_hits(struct symbol *sym, struct evsel *evsel)
> {
> + int evidx = evsel->core.idx;
> struct annotation *notes = symbol__annotation(sym);
> - struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
> + struct sym_hist *h = annotation__histogram(notes, evidx);
> u64 len = symbol__size(sym), offset;
>
> - for (offset = 0; offset < len; ++offset)
> - if (h->addr[offset].nr_samples != 0)
> + for (offset = 0; offset < len; ++offset) {
> + struct sym_hist_entry *entry;
> +
> + entry = annotated_source__hist_entry(notes->src, evidx, offset);
> + if (entry && entry->nr_samples != 0)
> printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2,
> - sym->start + offset, h->addr[offset].nr_samples);
> + sym->start + offset, entry->nr_samples);
> + }
> printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->nr_samples", h->nr_samples);
> }
>
> @@ -2855,8 +2865,14 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx)
>
> h->nr_samples = 0;
> for (offset = 0; offset < len; ++offset) {
> - h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8;
> - h->nr_samples += h->addr[offset].nr_samples;
> + struct sym_hist_entry *entry;
> +
> + entry = annotated_source__hist_entry(notes->src, evidx, offset);
> + if (entry == NULL)
> + continue;
> +
> + entry->nr_samples = entry->nr_samples * 7 / 8;
> + h->nr_samples += entry->nr_samples;
> }
> }
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index a2b0c8210740..3362980a5d3d 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -356,6 +356,17 @@ static inline struct sym_hist *annotation__histogram(struct annotation *notes, i
> return annotated_source__histogram(notes->src, idx);
> }
>
> +static inline struct sym_hist_entry *
> +annotated_source__hist_entry(struct annotated_source *src, int idx, u64 offset)
> +{
> + struct sym_hist_entry *entry;
> + long key = offset << 16 | idx;
> +
> + if (!hashmap__find(src->samples, key, &entry))

Hmm.. then I've realized that it requires the header file anyway.
This code is needed by multiple places for stdio, tui, gtk output.

Thanks,
Namhyung


> + return NULL;
> + return entry;
> +}
> +
> static inline struct annotation *symbol__annotation(struct symbol *sym)
> {
> return (void *)sym - symbol_conf.priv_size;
> --
> 2.44.0.rc1.240.g4c46232300-goog
>
>