[2/4] perf annotate: Calculate instruction overhead using hashmap

Message ID 20240228005230.287113-3-namhyung@kernel.org
State New
Headers
Series perf annotate: Improve memory usage for symbol histogram |

Commit Message

Namhyung Kim Feb. 28, 2024, 12:52 a.m. UTC
  Use annotated_source.samples hashmap instead of addr array in the
struct sym_hist.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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(-)
  

Comments

Namhyung Kim Feb. 28, 2024, 5:43 a.m. UTC | #1
On Tue, Feb 27, 2024 at 5:27 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Use annotated_source.samples hashmap instead of addr array in the
> struct sym_hist.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  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
>
>
  

Patch

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;